[clang-tools-extra] r353926 - [clangd] Handle a few more diag kinds in include fixer.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Feb 13 00:58:54 2019
New Revision: 353926

URL: http://llvm.org/viewvc/llvm-project?rev=353926&view=rev
Log:
[clangd] Handle a few more diag kinds in include fixer.

Reviewers: sammccall

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/IncludeFixer.cpp
clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp

Modified: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/IncludeFixer.cpp?rev=353926&r1=353925&r2=353926&view=diff
==
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp Wed Feb 13 00:58:54 2019
@@ -79,6 +79,12 @@ std::vector IncludeFixer::fix(Diagn
   case diag::err_typename_nested_not_found:
   case diag::err_no_template:
   case diag::err_no_template_suggest:
+  case diag::err_undeclared_use:
+  case diag::err_undeclared_use_suggest:
+  case diag::err_undeclared_var_use:
+  case diag::err_undeclared_var_use_suggest:
+  case diag::err_no_member: // Could be no member in namespace.
+  case diag::err_no_member_suggest:
 if (LastUnresolvedName) {
   // Try to fix unresolved name caused by missing declaraion.
   // E.g.

Modified: clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp?rev=353926&r1=353925&r2=353926&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp Wed Feb 13 
00:58:54 2019
@@ -368,11 +368,14 @@ TEST(IncludeFixerTest, Typo) {
   Annotations Test(R"cpp(
 $insert[[]]namespace ns {
 void foo() {
-  $unqualified[[X]] x;
+  $unqualified1[[X]] x;
+  $unqualified2[[X]]::Nested n;
 }
 }
 void bar() {
-  ns::$qualified[[X]] x; // ns:: is valid.
+  ns::$qualified1[[X]] x; // ns:: is valid.
+  ns::$qualified2[[X]](); // Error: no member in namespace
+
   ::$global[[Global]] glob;
 }
   )cpp");
@@ -385,13 +388,21 @@ void bar() {
   EXPECT_THAT(
   TU.build().getDiagnostics(),
   UnorderedElementsAre(
-  AllOf(Diag(Test.range("unqualified"), "unknown type name 'X'"),
+  AllOf(Diag(Test.range("unqualified1"), "unknown type name 'X'"),
+WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+"Add include \"x.h\" for symbol ns::X"))),
+  AllOf(Diag(Test.range("unqualified2"),
+ "use of undeclared identifier 'X'"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),
-  AllOf(Diag(Test.range("qualified"),
+  AllOf(Diag(Test.range("qualified1"),
  "no type named 'X' in namespace 'ns'"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),
+  AllOf(Diag(Test.range("qualified2"),
+ "no member named 'X' in namespace 'ns'"),
+WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+"Add include \"x.h\" for symbol ns::X"))),
   AllOf(Diag(Test.range("global"),
  "no type named 'Global' in the global namespace"),
 WithFix(Fix(Test.range("insert"), "#include \"global.h\"\n",


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

[PATCH] D58135: [clangd] Handle a few more diag kinds in include fixer.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353926: [clangd] Handle a few more diag kinds in include 
fixer. (authored by ioeric, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58135

Files:
  clang-tools-extra/trunk/clangd/IncludeFixer.cpp
  clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp


Index: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
===
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp
@@ -79,6 +79,12 @@
   case diag::err_typename_nested_not_found:
   case diag::err_no_template:
   case diag::err_no_template_suggest:
+  case diag::err_undeclared_use:
+  case diag::err_undeclared_use_suggest:
+  case diag::err_undeclared_var_use:
+  case diag::err_undeclared_var_use_suggest:
+  case diag::err_no_member: // Could be no member in namespace.
+  case diag::err_no_member_suggest:
 if (LastUnresolvedName) {
   // Try to fix unresolved name caused by missing declaraion.
   // E.g.
Index: clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
@@ -368,11 +368,14 @@
   Annotations Test(R"cpp(
 $insert[[]]namespace ns {
 void foo() {
-  $unqualified[[X]] x;
+  $unqualified1[[X]] x;
+  $unqualified2[[X]]::Nested n;
 }
 }
 void bar() {
-  ns::$qualified[[X]] x; // ns:: is valid.
+  ns::$qualified1[[X]] x; // ns:: is valid.
+  ns::$qualified2[[X]](); // Error: no member in namespace
+
   ::$global[[Global]] glob;
 }
   )cpp");
@@ -385,13 +388,21 @@
   EXPECT_THAT(
   TU.build().getDiagnostics(),
   UnorderedElementsAre(
-  AllOf(Diag(Test.range("unqualified"), "unknown type name 'X'"),
+  AllOf(Diag(Test.range("unqualified1"), "unknown type name 'X'"),
+WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+"Add include \"x.h\" for symbol ns::X"))),
+  AllOf(Diag(Test.range("unqualified2"),
+ "use of undeclared identifier 'X'"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),
-  AllOf(Diag(Test.range("qualified"),
+  AllOf(Diag(Test.range("qualified1"),
  "no type named 'X' in namespace 'ns'"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),
+  AllOf(Diag(Test.range("qualified2"),
+ "no member named 'X' in namespace 'ns'"),
+WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+"Add include \"x.h\" for symbol ns::X"))),
   AllOf(Diag(Test.range("global"),
  "no type named 'Global' in the global namespace"),
 WithFix(Fix(Test.range("insert"), "#include \"global.h\"\n",


Index: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
===
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp
@@ -79,6 +79,12 @@
   case diag::err_typename_nested_not_found:
   case diag::err_no_template:
   case diag::err_no_template_suggest:
+  case diag::err_undeclared_use:
+  case diag::err_undeclared_use_suggest:
+  case diag::err_undeclared_var_use:
+  case diag::err_undeclared_var_use_suggest:
+  case diag::err_no_member: // Could be no member in namespace.
+  case diag::err_no_member_suggest:
 if (LastUnresolvedName) {
   // Try to fix unresolved name caused by missing declaraion.
   // E.g.
Index: clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
@@ -368,11 +368,14 @@
   Annotations Test(R"cpp(
 $insert[[]]namespace ns {
 void foo() {
-  $unqualified[[X]] x;
+  $unqualified1[[X]] x;
+  $unqualified2[[X]]::Nested n;
 }
 }
 void bar() {
-  ns::$qualified[[X]] x; // ns:: is valid.
+  ns::$qualified1[[X]] x; // ns:: is valid.
+  ns::$qualified2[[X]](); // Error: no member in namespace
+
   ::$global[[Global]] glob;
 }
   )cpp");
@@ -385,13 +388,21 @@
   EXPECT_THAT(
   TU.build().getDiagnostics(),
   UnorderedElementsAre(
-  AllOf(Diag(Test.range("unqualified"), "unknown type name 'X'"),
+  AllOf(Diag(Test.range("unqualified1"), "unknown type name 'X'"),
+WithFix(Fix(Test.range("i

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

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

Szelethus added a comment.

@rnkovacs and @xazax.hun might be able to help with that?


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] D56370: [clangd] Add support for type hierarchy (super types only for now)

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:543
+  return CB(InpAST.takeError());
+CB(clangd::getTypeHierarchy(InpAST->AST, Item.range.start, Resolve,
+Direction));

nridge wrote:
> sammccall wrote:
> > nridge wrote:
> > > sammccall wrote:
> > > > relying on the item range to resolve the actual symbol isn't reliable:
> > > >  - the source code may have changed
> > > >  - the range may not be within the TU, and might be e.g. in an indexed 
> > > > header we don't have a compile command for.
> > > Good point. I appreciate a bit better now why you suggested trying to 
> > > avoid `resolve` for now :)
> > > 
> > > What do you think of the following implementation approach:
> > > 
> > >  * Use the `data` field of `TypeHierarchyItem` (which the client will 
> > > send back to the server for `resolve` queries) to send the `SymbolID` of 
> > > the item to the client.
> > >  * When the client sends back the `SymbolID` in the `resolve` request, 
> > > look up the symbol in the index, and use the source range from the symbol.
> > > 
> > > (Later, when we start storing base <--> derived relationships in the 
> > > index for subtype support, we could just answer the entire `resolve` 
> > > query using the index.)
> > Thanks for exploring all the options here!
> > 
> > Generally we've tried to avoid relying on the index unless it's needed, 
> > using the AST where possible. There are failure modes here:
> >  - the base type is in the current file, which is actively edited. The 
> > ranges in the index may be off due to staleness.
> >  - the base type is not indexed, because it is e.g. a member class inside a 
> > class template
> >  - there's no index (`-index=0`, though I'm not sure why we still support 
> > this) or the index is stale and the type is missing (we're working on 
> > making index updates more async)
> >  - the base type is not encodable.
> > 
> > There are just more moving pieces here, I think. Is there a clear reason to 
> > support resolve for parents?
> > Is there a clear reason to support resolve for parents?
> 
> Just what I said earlier about a hypothetical client that relies on it.
> 
> However, given the complications involved in implementing it, I'm happy with 
> only being concerned with actual clients, not hypothetical ones. The only 
> client implementation I currently know of is Theia, and I checked that it 
> works fine without `resolve`, so I'm happy with deferring work on `resolve` 
> for now.
> 
> If another client comes along that relies on `resolve`, we can revisit this.
> Just what I said earlier about a hypothetical client that relies on it.

I'll try to get the spec clarified such that eager resolution is always fine.
But if we ever do need to deal with such clients, @ioeric had a neat idea: we 
can just stash the rest of the response in the `data` field, and then 
"resolution" can just pull it out. This should be easy to bolt on.



Comment at: clang-tools-extra/clangd/XRefs.cpp:842
+
+  StringRef Filename = SM.getFilename(BeginLoc);
+  std::string FileURI = toURI(SM, Filename, {});

This is more conversions than necessary.
I think we just need:

```auto FilePath = 
getCanonicalPath(SM.getFileEntryForID(SM.getFileID(BeginLoc)), SM);
auto TUPath = getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()));
if (!FilePath || !TUPath)
  return;
THI.uri = URIForFile::Canonicalize(*FilePath, *TUPath);
```

(This is still more lines than I'd like, maybe we should have some helper for 
going from (File ID, SourceManager) --> URI)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370



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

r353931 - Look through typedefs in getFunctionTypeWithExceptionSpec

[Stephan Bergmann via cfe-commits]

Author: sberg
Date: Wed Feb 13 01:39:17 2019
New Revision: 353931

URL: http://llvm.org/viewvc/llvm-project?rev=353931&view=rev
Log:
Look through typedefs in getFunctionTypeWithExceptionSpec

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

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

Added:
cfe/trunk/test/AST/function-alias.cpp
Modified:
cfe/trunk/lib/AST/ASTContext.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=353931&r1=353930&r2=353931&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Feb 13 01:39:17 2019
@@ -2771,7 +2771,7 @@ QualType ASTContext::getFunctionTypeWith
 
   // Anything else must be a function type. Rebuild it with the new exception
   // specification.
-  const auto *Proto = cast(Orig);
+  const auto *Proto = Orig->getAs();
   return getFunctionType(
   Proto->getReturnType(), Proto->getParamTypes(),
   Proto->getExtProtoInfo().withExceptionSpec(ESI));

Added: cfe/trunk/test/AST/function-alias.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/function-alias.cpp?rev=353931&view=auto
==
--- cfe/trunk/test/AST/function-alias.cpp (added)
+++ cfe/trunk/test/AST/function-alias.cpp Wed Feb 13 01:39:17 2019
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only %s
+
+// Verify that ASTContext::getFunctionTypeWithExceptionSpec (called through
+// ASTContext::hasSameFunctionTypeIgnoringExceptionSpec from
+// ExprEvaluatorBase::handleCallExpr in lib/AST/ExprConstant.cpp) does not 
crash
+// for a type alias.
+
+constexpr int f() noexcept { return 0; }
+
+using F = int();
+
+constexpr int g(F * p) { return p(); }
+
+constexpr int n = g(f);


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

[PATCH] D58062: Support framework import/include auto-completion

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Code looks good apart from one case where we encounter 
Foo.framework/Subdir1/Subdir2.

Can you add a test to `clang/test/CodeCompletion`? `included-files.cpp` has the 
existing tests, not sure if you can add them there or it needs to be an obj-c 
file for framework includes.




Comment at: lib/Sema/SemaCodeComplete.cpp:8404
+if (LookupType == DirectoryLookup::LT_Framework) {
+  if (!Filename.endswith(".framework")) {
+break;

nit: no {} needed for these one-line `if`s



Comment at: lib/Sema/SemaCodeComplete.cpp:8404
+if (LookupType == DirectoryLookup::LT_Framework) {
+  if (!Filename.endswith(".framework")) {
+break;

sammccall wrote:
> nit: no {} needed for these one-line `if`s
this check appears to be incorrect, it'll fire even if NativeRelDir is nonempty 
(i.e. a we're inside a framework already.)
I think this can be combined with the one below it, see next comment.



Comment at: lib/Sema/SemaCodeComplete.cpp:8407
+  }
+  if (NativeRelDir.empty()) {
+Filename.consume_back(".framework");

`consume_back` checks for the suffix, just `if (NativeRelDir.empty() && 
!Filename.consume_back(".framework")) break;` is enough


Repository:
  rC Clang

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

https://reviews.llvm.org/D58062



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

[PATCH] D58056: Look through typedefs in getFunctionTypeWithExceptionSpec

[Stephan Bergmann via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC353931: Look through typedefs in 
getFunctionTypeWithExceptionSpec (authored by sberg, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58056?vs=186269&id=186604#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58056

Files:
  lib/AST/ASTContext.cpp
  test/AST/function-alias.cpp


Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -2771,7 +2771,7 @@
 
   // Anything else must be a function type. Rebuild it with the new exception
   // specification.
-  const auto *Proto = cast(Orig);
+  const auto *Proto = Orig->getAs();
   return getFunctionType(
   Proto->getReturnType(), Proto->getParamTypes(),
   Proto->getExtProtoInfo().withExceptionSpec(ESI));
Index: test/AST/function-alias.cpp
===
--- test/AST/function-alias.cpp
+++ test/AST/function-alias.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only %s
+
+// Verify that ASTContext::getFunctionTypeWithExceptionSpec (called through
+// ASTContext::hasSameFunctionTypeIgnoringExceptionSpec from
+// ExprEvaluatorBase::handleCallExpr in lib/AST/ExprConstant.cpp) does not 
crash
+// for a type alias.
+
+constexpr int f() noexcept { return 0; }
+
+using F = int();
+
+constexpr int g(F * p) { return p(); }
+
+constexpr int n = g(f);


Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -2771,7 +2771,7 @@
 
   // Anything else must be a function type. Rebuild it with the new exception
   // specification.
-  const auto *Proto = cast(Orig);
+  const auto *Proto = Orig->getAs();
   return getFunctionType(
   Proto->getReturnType(), Proto->getParamTypes(),
   Proto->getExtProtoInfo().withExceptionSpec(ESI));
Index: test/AST/function-alias.cpp
===
--- test/AST/function-alias.cpp
+++ test/AST/function-alias.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only %s
+
+// Verify that ASTContext::getFunctionTypeWithExceptionSpec (called through
+// ASTContext::hasSameFunctionTypeIgnoringExceptionSpec from
+// ExprEvaluatorBase::handleCallExpr in lib/AST/ExprConstant.cpp) does not crash
+// for a type alias.
+
+constexpr int f() noexcept { return 0; }
+
+using F = int();
+
+constexpr int g(F * p) { return p(); }
+
+constexpr int n = g(f);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58056: Look through typedefs in getFunctionTypeWithExceptionSpec

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

committed for now to get the crash fixed; if there are issues with the test 
they can be addressed later


Repository:
  rC Clang

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

https://reviews.llvm.org/D58056



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 186605.
glider marked 4 inline comments as done.
glider added a comment.

Addressed the comments.


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

https://reviews.llvm.org/D57898

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

Index: tools/clang/test/CodeGenCXX/auto-var-init.cpp
===
--- tools/clang/test/CodeGenCXX/auto-var-init.cpp
+++ tools/clang/test/CodeGenCXX/auto-var-init.cpp
@@ -1,6 +1,8 @@
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks %s -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s -check-prefix=PATTERN
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s -check-prefix=ZERO
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s -check-prefixes=PATTERN,PATTERN-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -O1 -emit-llvm -o - | FileCheck %s -check-prefixes=PATTERN,PATTERN-O1
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s -check-prefixes=ZERO,ZERO-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -O1 -emit-llvm -o - | FileCheck %s -check-prefixes=ZERO,ZERO-O1
 
 template void used(T &) noexcept;
 
@@ -30,104 +32,104 @@
 // PATTERN-NOT: undef
 // ZERO-NOT: undef
 
-// PATTERN: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
 struct empty {};
-// PATTERN: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
-// PATTERN: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
-// ZERO: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// PATTERN-O0: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// ZERO-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
 struct small { char c; };
-// PATTERN: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
 struct smallinit { char c = 42; };
-// PATTERN: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
 struct smallpartinit { char c = 42, d; };
-// PATTERN: @__const.test_nullinit_uninit.uninit = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, align 8
-// PATTERN: @__const.test_nullinit_braces.braces = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, align 8
-// PATTERN: @__const.test_nullinit_custom.custom = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, align 8
+// PATTERN-O0: @__const.test_nullinit_uninit.uninit = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, align

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:979
+  if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+return false;
+  if (GlobalSize <= SizeLimit)

jfb wrote:
> The general 64-byte heuristic is fine with me. It's just a bit weird to 
> special-case `-O0`, but we do it elsewhere too (and it keeps the current 
> status-quo of letting the backend decide what to do). From that perspective 
> I'm fine with it.
> 
> @rjmccall do you have a strong preference either way?
> 
> One small nit: can you use `ByteSize` instead of just `Size`? Makes it easier 
> to figure out what's going on in code IMO.
I don't have a strong opinion on variable naming, but this:
```
 974 static bool shouldSplitStructStore(CodeGenModule &CGM,
 975uint64_t GlobalByteSize) {
 976   // Don't break structures that occupy more than one cacheline.
 977   uint64_t ByteSizeLimit = 64;
 978   if (CGM.getCodeGenOpts().OptimizationLevel == 0)
 979 return false;
 980   if (GlobalByteSize <= ByteSizeLimit)
 981 return true;
 982   return false;
```
looks a bit more heavyweight than the previous function, in which we also have 
`GlobalSize` and `SizeLimit`.



Comment at: tools/clang/test/CodeGenCXX/auto-var-init.cpp:476
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
 // PATTERN-LABEL: @test_empty_uninit()
+// PATTERN-O0: call void @llvm.memcpy{{.*}} @__const.test_empty_uninit.uninit

jfb wrote:
> The tests aren't matching labels anymore: `PATTERN-LABEL` is a dead label 
> check now. I think forking things at `-O0` and `-O1` is fine, but I think you 
> want a small `-O0` test (separate from this patch) which does one or two 
> things, and then you want this test to only look at `-O1`. That way you don't 
> need so much stuff changing in the test (and `-O0` is still tested).
Why? We're passing both PATTERN and PATTERN-OX to FileCheck when testing.
I've checked that replacing `@test_empty_uninit()` on this line with a 
different function name makes the test fail.


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

https://reviews.llvm.org/D57898



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

[PATCH] D57655: clang-format with UseTab: Always sometimes doesn't insert the right amount of tabs.

[Alexander Kornienko via Phabricator via cfe-commits]

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

Seems reasonable. LG with a couple of nits. Please let me know if you need to 
commit this for you.




Comment at: D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp:685
+  Text.append(Spaces, ' ');
+} else {
+  // Align to next tab.

nit: I'd just `break` here and remove the `else`.



Comment at: D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp:686
+} else {
+  // Align to next tab.
   Spaces -= FirstTabWidth;

nit: "Align to the next tab."


Repository:
  rC Clang

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

https://reviews.llvm.org/D57655



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Well, now we somewhat break padding initialization.

E.g. for the following struct:

  struct s { 
int a;
char b;
long int c;
  };

we generate the following constant initializer with `-O0`:

  .L__const.foo.local:
  .long   2863311530  # 0x
  .byte   170 # 0xaa
  .zero   3   
  .quad   -6148914691236517206# 0x
  .size   .L__const.foo.local, 16

, which overwrites the padding bytes on stack, but with a wrong constant.

OTOH with `-O1` (i.e. with my patch) we generate the following sequence of 
stores:

  movl$-1431655766, 8(%rsp)   # imm = 0x
  movb$-86, 12(%rsp)
  movabsq $-6148914691236517206, %rax # imm = 0x
  movq%rax, 16(%rsp)


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

https://reviews.llvm.org/D57898



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

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:842
+
+  StringRef Filename = SM.getFilename(BeginLoc);
+  std::string FileURI = toURI(SM, Filename, {});

sammccall wrote:
> This is more conversions than necessary.
> I think we just need:
> 
> ```auto FilePath = 
> getCanonicalPath(SM.getFileEntryForID(SM.getFileID(BeginLoc)), SM);
> auto TUPath = getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()));
> if (!FilePath || !TUPath)
>   return;
> THI.uri = URIForFile::Canonicalize(*FilePath, *TUPath);
> ```
> 
> (This is still more lines than I'd like, maybe we should have some helper for 
> going from (File ID, SourceManager) --> URI)
(I looked at adding such a helper, and I don't think it's a good idea - the few 
existing callsites that could use it currently benefit significantly from 
hoisting the getCanonicalPath from the TUPath outside of a loop. It may or may 
not be worth doing that here)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

... which happily skips the padding.

It occurs to me that we need to properly handle the padding in `patternFor` 
before we'll be able to split the structures.


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

https://reviews.llvm.org/D57898



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

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro updated this revision to Diff 186611.
djtodoro added a comment.

- Rename: `VariableNotChanged `===> `ArgumentNotModified`
- Refactor a test case


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

https://reviews.llvm.org/D58035

Files:
  include/clang/AST/Decl.h
  lib/CodeGen/CGDebugInfo.cpp
  lib/Sema/SemaExpr.cpp
  test/CodeGen/debug-info-varchange.c

Index: test/CodeGen/debug-info-varchange.c
===
--- /dev/null
+++ test/CodeGen/debug-info-varchange.c
@@ -0,0 +1,35 @@
+// RUN: %clang -femit-param-entry-values -emit-llvm -S -g %s -o - | FileCheck %s
+
+// CHECK: !DILocalVariable(name: "a", arg: 1, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
+// CHECK: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})
+// CHECK: !DILocalVariable(name: "test_s", arg: 3, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
+// CHECK: !DILocalVariable(name: "test_s2", arg: 4, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})
+// CHECK: !DILocalVariable(name: "x", scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})
+// CHECK: !DILocalVariable(name: "y", scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})
+
+typedef struct s {
+  int m;
+  int n;
+}S;
+
+void foo (int a, int b,
+  S test_s, S test_s2)
+{
+  b++;
+  b = a + 1;
+  if (b>4)
+test_s2.m = 434;
+}
+
+int main()
+{
+  S test_s = {4, 5};
+
+  int x = 5;
+  int y = 6;
+
+  foo(x , y, test_s, test_s);
+
+  return 0;
+}
+
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -16,6 +16,7 @@
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ASTMutationListener.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
@@ -11276,6 +11277,19 @@
 DiagnoseConstAssignment(S, E, Loc);
 }
 
+/// Argument's value might be modified, so update the info.
+static void EmitArgumentsValueModification(Expr *E) {
+  if (const DeclRefExpr *LHSDeclRef =
+  dyn_cast(E->IgnoreParenCasts()))
+if (const ValueDecl *Decl = LHSDeclRef->getDecl())
+  if (const VarDecl *VD = dyn_cast(Decl)) {
+if (!isa(VD))
+  return;
+if (!VD->getIsArgumentModified())
+  VD->setIsArgumentModified();
+  }
+}
+
 /// CheckForModifiableLvalue - Verify that E is a modifiable lvalue.  If not,
 /// emit an error and return true.  If so, return false.
 static bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) {
@@ -11283,6 +11297,12 @@
 
   S.CheckShadowingDeclModification(E, Loc);
 
+  if (MemberExpr *ME = dyn_cast(E)) {
+if (Expr *BaseExpr = ME->getBase())
+  EmitArgumentsValueModification(BaseExpr);
+  } else
+EmitArgumentsValueModification(E);
+
   SourceLocation OrigLoc = Loc;
   Expr::isModifiableLvalueResult IsLV = E->isModifiableLvalue(S.Context,
   &Loc);
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -3778,6 +3778,11 @@
   if (VD->isImplicit())
 Flags |= llvm::DINode::FlagArtificial;
 
+  auto &CGOpts = CGM.getCodeGenOpts();
+  if (CGOpts.EnableParamEntryValues && !VD->getIsArgumentModified())
+Flags |= llvm::DINode::FlagArgumentNotModified;
+
+
   auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
 
   unsigned AddressSpace = CGM.getContext().getTargetAddressSpace(VD->getType());
Index: include/clang/AST/Decl.h
===
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -840,6 +840,8 @@
   /// It is illegal to call this function with SC == None.
   static const char *getStorageClassSpecifierString(StorageClass SC);
 
+  mutable bool IsArgumentModified = false;
+
 protected:
   // A pointer union of Stmt * and EvaluatedStmt *. When an EvaluatedStmt, we
   // have allocated the auxiliary struct of information there.
@@ -1477,6 +1479,13 @@
   /// Do we need to emit an exit-time destructor for this variable?
   bool isNoDestroy(const ASTContext &) const;
 
+  bool getIsArgumentModified() const {
+return IsArgumentModified;
+  }
+  void setIsArgumentModified() const {
+IsArgumentModified = true;
+  }
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K >= firstVar && K <= lastVar; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r350404 - Refactor the way we handle diagnosing unused expression results.

[Hans Wennborg via cfe-commits]

Reverted on the release_80 branch in r353935.

On Fri, Jan 4, 2019 at 6:01 PM Aaron Ballman via cfe-commits
 wrote:
>
> Author: aaronballman
> Date: Fri Jan  4 08:58:14 2019
> New Revision: 350404
>
> URL: http://llvm.org/viewvc/llvm-project?rev=350404&view=rev
> Log:
> Refactor the way we handle diagnosing unused expression results.
>
> Rather than sprinkle calls to DiagnoseUnusedExprResult() around in places 
> where we want diagnostics, we now diagnose unused expression statements and 
> full expressions in a more generic way when acting on the final expression 
> statement. This results in more appropriate diagnostics for [[nodiscard]] 
> where we were previously lacking them, such as when the body of a for loop is 
> not a compound statement.
>
> This patch fixes PR39837.
>
> Modified:
> cfe/trunk/include/clang/Parse/Parser.h
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/Parse/ParseObjc.cpp
> cfe/trunk/lib/Parse/ParseOpenMP.cpp
> cfe/trunk/lib/Parse/ParseStmt.cpp
> cfe/trunk/lib/Sema/SemaCoroutine.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/lib/Sema/SemaExprCXX.cpp
> cfe/trunk/lib/Sema/SemaLambda.cpp
> cfe/trunk/lib/Sema/SemaOpenMP.cpp
> cfe/trunk/lib/Sema/SemaStmt.cpp
> cfe/trunk/lib/Sema/TreeTransform.h
> cfe/trunk/test/CXX/stmt.stmt/stmt.select/p3.cpp
> cfe/trunk/test/CodeCompletion/pragma-macro-token-caching.c
> cfe/trunk/test/Parser/cxx1z-init-statement.cpp
> cfe/trunk/test/Parser/switch-recovery.cpp
> cfe/trunk/test/SemaCXX/cxx1z-init-statement.cpp
> cfe/trunk/test/SemaCXX/for-range-examples.cpp
> cfe/trunk/test/SemaCXX/warn-unused-result.cpp
>
> Modified: cfe/trunk/include/clang/Parse/Parser.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=350404&r1=350403&r2=350404&view=diff
> ==
> --- cfe/trunk/include/clang/Parse/Parser.h (original)
> +++ cfe/trunk/include/clang/Parse/Parser.h Fri Jan  4 08:58:14 2019
> @@ -360,6 +360,11 @@ class Parser : public CodeCompletionHand
>/// just a regular sub-expression.
>SourceLocation ExprStatementTokLoc;
>
> +  /// Tests whether an expression value is discarded based on token 
> lookahead.
> +  /// It will return true if the lexer is currently processing the })
> +  /// terminating a GNU statement expression and false otherwise.
> +  bool isExprValueDiscarded();
> +
>  public:
>Parser(Preprocessor &PP, Sema &Actions, bool SkipFunctionBodies);
>~Parser() override;
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=350404&r1=350403&r2=350404&view=diff
> ==
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Fri Jan  4 08:58:14 2019
> @@ -1365,6 +1365,7 @@ public:
>void PopCompoundScope();
>
>sema::CompoundScopeInfo &getCurCompoundScope() const;
> +  bool isCurCompoundStmtAStmtExpr() const;
>
>bool hasAnyUnrecoverableErrorsInThisFunction() const;
>
> @@ -3685,16 +3686,17 @@ public:
>  return MakeFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation());
>}
>FullExprArg MakeFullExpr(Expr *Arg, SourceLocation CC) {
> -return FullExprArg(ActOnFinishFullExpr(Arg, CC).get());
> +return FullExprArg(
> +ActOnFinishFullExpr(Arg, CC, /*DiscardedValue*/ false).get());
>}
>FullExprArg MakeFullDiscardedValueExpr(Expr *Arg) {
>  ExprResult FE =
> -  ActOnFinishFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation(),
> -  /*DiscardedValue*/ true);
> +ActOnFinishFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation(),
> +/*DiscardedValue*/ true);
>  return FullExprArg(FE.get());
>}
>
> -  StmtResult ActOnExprStmt(ExprResult Arg);
> +  StmtResult ActOnExprStmt(ExprResult Arg, bool DiscardedValue = true);
>StmtResult ActOnExprStmtError();
>
>StmtResult ActOnNullStmt(SourceLocation SemiLoc,
> @@ -5340,13 +5342,12 @@ public:
>CreateMaterializeTemporaryExpr(QualType T, Expr *Temporary,
>   bool BoundToLvalueReference);
>
> -  ExprResult ActOnFinishFullExpr(Expr *Expr) {
> -return ActOnFinishFullExpr(Expr, Expr ? Expr->getExprLoc()
> -  : SourceLocation());
> +  ExprResult ActOnFinishFullExpr(Expr *Expr, bool DiscardedValue) {
> +return ActOnFinishFullExpr(
> +Expr, Expr ? Expr->getExprLoc() : SourceLocation(), DiscardedValue);
>}
>ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC,
> - bool DiscardedValue = false,
> - bool IsConstexpr = false);
> + boo

r353943 - [Analyzer] Crash fix for FindLastStoreBRVisitor

[Adam Balogh via cfe-commits]

Author: baloghadamsoftware
Date: Wed Feb 13 04:25:47 2019
New Revision: 353943

URL: http://llvm.org/viewvc/llvm-project?rev=353943&view=rev
Log:
[Analyzer] Crash fix for FindLastStoreBRVisitor

FindLastStoreBRVisitor tries to find the first node in the exploded graph where
the current value was assigned to a region. This node is called the "store
site". It is identified by a pair of Pred and Succ nodes where Succ already has
the binding for the value while Pred does not have it. However the visitor
mistakenly identifies a node pair as the store site where the value is a
`LazyCompoundVal` and `Pred` does not have a store yet but `Succ` has it. In
this case the `LazyCompoundVal` is different in the `Pred` node because it also
contains the store which is different in the two nodes. This error may lead to
crashes (a declaration is cast to a parameter declaration without check) or
misleading bug path notes.

In this patch we fix this problem by checking for unequal `LazyCompoundVals`: if
their region is equal, and their store is the same as the store of their nodes
we consider them as equal when looking for the "store site". This is an
approximation because we do not check for differences of the subvalues
(structure members or array elements) in the stores.

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


Added:
cfe/trunk/test/Analysis/PR40625.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/uninit-vals.m

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=353943&r1=353942&r2=353943&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Wed Feb 13 
04:25:47 2019
@@ -153,6 +153,32 @@ const Expr *bugreporter::getDerefExpr(co
   return E;
 }
 
+/// Comparing internal representations of symbolic values (via
+/// SVal::operator==()) is a valid way to check if the value was updated,
+/// unless it's a LazyCompoundVal that may have a different internal
+/// representation every time it is loaded from the state. In this function we
+/// do an approximate comparison for lazy compound values, checking that they
+/// are the immediate snapshots of the tracked region's bindings within the
+/// node's respective states but not really checking that these snapshots
+/// actually contain the same set of bindings.
+bool hasVisibleUpdate(const ExplodedNode *LeftNode, SVal LeftVal,
+  const ExplodedNode *RightNode, SVal RightVal) {
+  if (LeftVal == RightVal)
+return true;
+
+  const auto LLCV = LeftVal.getAs();
+  if (!LLCV)
+return false;
+
+  const auto RLCV = RightVal.getAs();
+  if (!RLCV)
+return false;
+
+  return LLCV->getRegion() == RLCV->getRegion() &&
+LLCV->getStore() == LeftNode->getState()->getStore() &&
+RLCV->getStore() == RightNode->getState()->getStore();
+}
+
 
//===--===//
 // Definitions for bug reporter visitors.
 
//===--===//
@@ -1177,7 +1203,7 @@ FindLastStoreBRVisitor::VisitNode(const
 if (Succ->getState()->getSVal(R) != V)
   return nullptr;
 
-if (Pred->getState()->getSVal(R) == V) {
+if (hasVisibleUpdate(Pred, Pred->getState()->getSVal(R), Succ, V)) {
   Optional PS = Succ->getLocationAs();
   if (!PS || PS->getLocationValue() != R)
 return nullptr;
@@ -1198,6 +1224,7 @@ FindLastStoreBRVisitor::VisitNode(const
 // UndefinedVal.)
 if (Optional CE = Succ->getLocationAs()) {
   if (const auto *VR = dyn_cast(R)) {
+
 const auto *Param = cast(VR->getDecl());
 
 ProgramStateManager &StateMgr = BRC.getStateManager();

Added: cfe/trunk/test/Analysis/PR40625.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/PR40625.cpp?rev=353943&view=auto
==
--- cfe/trunk/test/Analysis/PR40625.cpp (added)
+++ cfe/trunk/test/Analysis/PR40625.cpp Wed Feb 13 04:25:47 2019
@@ -0,0 +1,16 @@
+// RUN: %clang_analyze_cc1 -std=c++11 
-analyzer-checker=core,alpha.core.CallAndMessageUnInitRefArg  %s -verify
+
+void f(const int *end);
+
+void g(const int (&arrr)[10]) {
+  f(arrr+sizeof(arrr)); // expected-warning{{1st function call argument is a 
pointer to uninitialized value}}
+  // FIXME: This is a false positive that should be fixed. Until then this
+  //tests the crash fix in FindLastStoreBRVisitor (beside
+  //uninit-vals.m).
+}
+
+void h() {
+  int arr[10];
+
+  g(arr);
+}

Modified: cfe/trunk/test/Analysis/uninit-vals.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/uninit-vals.m?rev=353943&r1=353942&r2=353943&view=di

[PATCH] D58067: [Analyzer] Crash fix for FindLastStoreBRVisitor

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353943: [Analyzer] Crash fix for FindLastStoreBRVisitor 
(authored by baloghadamsoftware, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58067?vs=186470&id=186625#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58067

Files:
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  test/Analysis/PR40625.cpp
  test/Analysis/uninit-vals.m

Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -153,6 +153,32 @@
   return E;
 }
 
+/// Comparing internal representations of symbolic values (via
+/// SVal::operator==()) is a valid way to check if the value was updated,
+/// unless it's a LazyCompoundVal that may have a different internal
+/// representation every time it is loaded from the state. In this function we
+/// do an approximate comparison for lazy compound values, checking that they
+/// are the immediate snapshots of the tracked region's bindings within the
+/// node's respective states but not really checking that these snapshots
+/// actually contain the same set of bindings.
+bool hasVisibleUpdate(const ExplodedNode *LeftNode, SVal LeftVal,
+  const ExplodedNode *RightNode, SVal RightVal) {
+  if (LeftVal == RightVal)
+return true;
+
+  const auto LLCV = LeftVal.getAs();
+  if (!LLCV)
+return false;
+
+  const auto RLCV = RightVal.getAs();
+  if (!RLCV)
+return false;
+
+  return LLCV->getRegion() == RLCV->getRegion() &&
+LLCV->getStore() == LeftNode->getState()->getStore() &&
+RLCV->getStore() == RightNode->getState()->getStore();
+}
+
 //===--===//
 // Definitions for bug reporter visitors.
 //===--===//
@@ -1177,7 +1203,7 @@
 if (Succ->getState()->getSVal(R) != V)
   return nullptr;
 
-if (Pred->getState()->getSVal(R) == V) {
+if (hasVisibleUpdate(Pred, Pred->getState()->getSVal(R), Succ, V)) {
   Optional PS = Succ->getLocationAs();
   if (!PS || PS->getLocationValue() != R)
 return nullptr;
@@ -1198,6 +1224,7 @@
 // UndefinedVal.)
 if (Optional CE = Succ->getLocationAs()) {
   if (const auto *VR = dyn_cast(R)) {
+
 const auto *Param = cast(VR->getDecl());
 
 ProgramStateManager &StateMgr = BRC.getStateManager();
Index: test/Analysis/PR40625.cpp
===
--- test/Analysis/PR40625.cpp
+++ test/Analysis/PR40625.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,alpha.core.CallAndMessageUnInitRefArg  %s -verify
+
+void f(const int *end);
+
+void g(const int (&arrr)[10]) {
+  f(arrr+sizeof(arrr)); // expected-warning{{1st function call argument is a pointer to uninitialized value}}
+  // FIXME: This is a false positive that should be fixed. Until then this
+  //tests the crash fix in FindLastStoreBRVisitor (beside
+  //uninit-vals.m).
+}
+
+void h() {
+  int arr[10];
+
+  g(arr);
+}
Index: test/Analysis/uninit-vals.m
===
--- test/Analysis/uninit-vals.m
+++ test/Analysis/uninit-vals.m
@@ -394,11 +394,11 @@
   struct {
 int : 4;
 int y : 4;
-  } a, b, c;
+  } a, b, c; // expected-note{{'c' initialized here}}
 
   a.y = 2;
 
-  b = a; // expected-note{{Value assigned to 'c'}}
+  b = a;
   clang_analyzer_eval(b.y == 2); // expected-warning{{TRUE}}
  // expected-note@-1{{TRUE}}
 
@@ -411,11 +411,11 @@
   struct {
 int x : 4;
 int : 4;
-  } a, b, c;
+  } a, b, c; // expected-note{{'c' initialized here}}
 
   a.x = 1;
 
-  b = a; // expected-note{{Value assigned to 'c'}}
+  b = a;
   clang_analyzer_eval(b.x == 1); // expected-warning{{TRUE}}
  // expected-note@-1{{TRUE}}
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58107: [MinGW] Add the profiling library when necessary

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added a comment.

This change breaks building/testing the compiler with `CLANG_DEFAULT_LINKER` 
set to `lld`. Was this intentional? What should people do if they want to use 
`CLANG_DEFAULT_LINKER` and run the test suite?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58107



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

[PATCH] D58157: Stop enabling clang-tools-extra automatically when clang is in LLVM_ENABLE_PROJECTS

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D58157#1395762 , @mehdi_amini wrote:

> In D58157#1395716 , @rnk wrote:
>
> > I think we have consensus,
>
>
> Based on three comments in a revision? Seems strange to me.
>  I don't really care about this, so do whatever you want, but I would expect 
> that "consensus" means an actual wider discussion (i.e. llvm-dev + cfe-dev).


Please cite said discussion for when you added this, as requested above. Else, 
I think this has seen more discussion than the change that is undoing. It also 
has the support of several folks very actively working on clang and 
clang-tools-extra.


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

https://reviews.llvm.org/D58157



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

[PATCH] D58107: [MinGW] Add the profiling library when necessary

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D58107#1396063 , @davezarzycki 
wrote:

> This change breaks building/testing the compiler with `CLANG_DEFAULT_LINKER` 
> set to `lld`. Was this intentional? What should people do if they want to use 
> `CLANG_DEFAULT_LINKER` and run the test suite?


It wasn't intentional. Originally I added `-fuse-ld=lld` to the test, but this 
broke buildbots that didn't have lld available. My fixup in rC353922 
 avoided this by removing the `-fuse-ld=lld` 
(where I noted in the commit message that I guess that it would break such 
configurations). At that point I thought the other tests in this same file also 
would be equally broken in such a setup then. But I see that most other tests 
use `-fuse-ld=ld`, to work around this. (This option doesn't check if the named 
tool exists, contrary to `-fuse-ld=`.)

But if a linker named `-ld` exists, that will be used instead, so I'll 
need to make the regex a bit looser as well. I'll commit a fix for this soon.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58107



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

r353946 - [test] Tweak driver test from r353917 and r353922 to pass with a nondefault CLANG_DEFAULT_LINKER

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Wed Feb 13 05:13:45 2019
New Revision: 353946

URL: http://llvm.org/viewvc/llvm-project?rev=353946&view=rev
Log:
[test] Tweak driver test from r353917 and r353922 to pass with a nondefault 
CLANG_DEFAULT_LINKER

Force -fuse-ld=ld, as some other tests in the same file do.

Loosen the regex matching the linker tool name as well, as this
can end up being -ld in case such a named tool exists.

Modified:
cfe/trunk/test/Driver/instrprof-ld.c

Modified: cfe/trunk/test/Driver/instrprof-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/instrprof-ld.c?rev=353946&r1=353945&r2=353946&view=diff
==
--- cfe/trunk/test/Driver/instrprof-ld.c (original)
+++ cfe/trunk/test/Driver/instrprof-ld.c Wed Feb 13 05:13:45 2019
@@ -123,9 +123,9 @@
 // CHECK-WINDOWS-X86-64: "{{.*}}clang_rt.profile-x86_64.lib"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target x86_64-mingw32 -fprofile-instr-generate \
+// RUN: -target x86_64-mingw32 -fprofile-instr-generate -fuse-ld=ld \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN:   | FileCheck --check-prefix=CHECK-MINGW-X86-64 %s
 //
-// CHECK-MINGW-X86-64: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-MINGW-X86-64: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-MINGW-X86-64: 
"{{.*}}/Inputs/resource_dir{{/|}}lib{{/|}}windows{{/|}}libclang_rt.profile-x86_64.a"


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

[PATCH] D57655: clang-format with UseTab: Always sometimes doesn't insert the right amount of tabs.

[Hylke Kleve via Phabricator via cfe-commits]

hyklv updated this revision to Diff 186629.
hyklv marked 2 inline comments as done.
hyklv added a comment.

Updated comment and added break;


Repository:
  rC Clang

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

https://reviews.llvm.org/D57655

Files:
  D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
  D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp


Index: D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp
===
--- D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp
+++ D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp
@@ -8739,6 +8739,9 @@
"\t\tparameter2); \\\n"
"\t}",
Tab);
+  verifyFormat("int a;\t  // x\n"
+   "int ; // x\n",
+   Tab);
 
   Tab.TabWidth = 4;
   Tab.IndentWidth = 8;
Index: D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
===
--- D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
+++ D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
@@ -679,11 +679,15 @@
   case FormatStyle::UT_Always: {
 unsigned FirstTabWidth =
 Style.TabWidth - WhitespaceStartColumn % Style.TabWidth;
-// Indent with tabs only when there's at least one full tab.
-if (FirstTabWidth + Style.TabWidth <= Spaces) {
-  Spaces -= FirstTabWidth;
-  Text.append("\t");
-}
+// Insert only spaces when we want to end up before the next tab.
+if (Spaces < FirstTabWidth || Spaces == 1) {
+  Text.append(Spaces, ' ');
+  break;
+} 
+// Align to the next tab.
+Spaces -= FirstTabWidth;
+Text.append("\t");
+
 Text.append(Spaces / Style.TabWidth, '\t');
 Text.append(Spaces % Style.TabWidth, ' ');
 break;


Index: D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp
===
--- D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp
+++ D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp
@@ -8739,6 +8739,9 @@
"\t\tparameter2); \\\n"
"\t}",
Tab);
+  verifyFormat("int a;\t  // x\n"
+   "int ; // x\n",
+   Tab);
 
   Tab.TabWidth = 4;
   Tab.IndentWidth = 8;
Index: D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
===
--- D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
+++ D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
@@ -679,11 +679,15 @@
   case FormatStyle::UT_Always: {
 unsigned FirstTabWidth =
 Style.TabWidth - WhitespaceStartColumn % Style.TabWidth;
-// Indent with tabs only when there's at least one full tab.
-if (FirstTabWidth + Style.TabWidth <= Spaces) {
-  Spaces -= FirstTabWidth;
-  Text.append("\t");
-}
+// Insert only spaces when we want to end up before the next tab.
+if (Spaces < FirstTabWidth || Spaces == 1) {
+  Text.append(Spaces, ' ');
+  break;
+} 
+// Align to the next tab.
+Spaces -= FirstTabWidth;
+Text.append("\t");
+
 Text.append(Spaces / Style.TabWidth, '\t');
 Text.append(Spaces % Style.TabWidth, ' ');
 break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57655: clang-format with UseTab: Always sometimes doesn't insert the right amount of tabs.

[Hylke Kleve via Phabricator via cfe-commits]

hyklv added a comment.

In D57655#1395924 , @alexfh wrote:

> Seems reasonable. LG with a couple of nits. Please let me know if you need to 
> commit this for you.


cool. could you commit the change for me?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57655



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

[PATCH] D53928: Enable builtins necessary for SLEEF [AArch64] vectorized trigonometry libm functions

[Renato Golin via Phabricator via cfe-commits]

rengolin added a comment.
Herald added a subscriber: jdoerfert.

Funny thing is, SVML is also only supported, AFAIK, for Intel. I agree that we 
should emit errors, but we should also emit a similar error on SVML.

I know it's not entirely relevant to this patch, but we should keep the 
behaviour consistent on all veclibs.

Also, can you add a test for the new error messages, please?

The new changes look good to me and don't substantially deviate from the 
previous, approved, patch, so LGTM after addressing the comments.

Thanks!




Comment at: lib/Frontend/CompilerInvocation.cpp:678
+  else
+Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args)
+  << Name;

This is not really invalid value for the flag, it's invalid architecture for 
the value.

I think there's already a string for that somewhere in Clang.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53928



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

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

[Lingda Li via Phabricator via cfe-commits]

lildmh added a reviewer: kkwli0.
lildmh marked 4 inline comments as done.
lildmh added inline comments.



Comment at: lib/Parse/ParseOpenMP.cpp:2144
+parseMapType(*this, Data);
 }
 if (Data.MapType == OMPC_MAP_unknown) {

kkwli0 wrote:
> Although it is an error situation, will we have different behavior?
> 
> ```
> ... map(xclose, to: x)
> ```
> 
> Previously, it always parses both modifier and type.  After the change, the 
> call of `parseMapType` is skipped.  If it `OMPC_MAP_unknown`, 
> `IsMapTypeImplicit` is set.
Hi Kelvin,

Thanks a lot for the review! It will not change the behavior of parsing 
modifiers other than mapper. `parseMapTypeModifiers` will only return true when 
it finds a invalid mapper for now.


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

https://reviews.llvm.org/D58074



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

[PATCH] D45978: dllexport const variables must have external linkage.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 186641.
zahiraam marked 5 inline comments as done.

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

https://reviews.llvm.org/D45978

Files:
  lib/Sema/SemaDecl.cpp
  test/CodeGen/dllexport-1.c
  test/Sema/dllexport-1.cpp
  test/Sema/dllexport-2.cpp


Index: test/Sema/dllexport-2.cpp
===
--- /dev/null
+++ test/Sema/dllexport-2.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+
+// Export const variable.
+
+__declspec(dllexport) int const j; // expected-error {{default initialization 
of an object of const type 'const int'}} // expected-error {{'j' must have 
external linkage when declared 'dllexport'}}
Index: test/Sema/dllexport-1.cpp
===
--- /dev/null
+++ test/Sema/dllexport-1.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+
+// Export const variable initialization.
+
+// expected-no-diagnostics
+__declspec(dllexport) int const x = 3;
Index: test/CodeGen/dllexport-1.c
===
--- /dev/null
+++ test/CodeGen/dllexport-1.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -emit-llvm -fms-extensions -o - %s | FileCheck %s
+
+
+// Export const variable.
+
+// CHECK: @x = dso_local dllexport constant i32 3, align 4
+// CHECK: @z = dso_local constant i32 4, align 4
+// CHECK: @y = common dso_local dllexport global i32 0, align 4
+
+__declspec(dllexport) int const x = 3;
+__declspec(dllexport) const int y;
+extern int const z = 4; // expected-warning{{'extern' variable has an 
initializer}}
+
+int main() {
+  int a = x + y + z;
+  return a;
+}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11367,6 +11367,16 @@
 !isTemplateInstantiation(VDecl->getTemplateSpecializationKind()))
   Diag(VDecl->getLocation(), diag::warn_extern_init);
 
+// In Microsoft C++ mode, a const variable defined in namespace scope has
+// external linkage by default if the variable is declared with
+// __declspec(dllexport).
+if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+getLangOpts().CPlusPlus &&
+VDecl->getType().isLocalConstQualified() &&
+VDecl->hasAttr() &&
+VDecl->getDefinition())
+  VDecl->setStorageClass(SC_Extern);
+
 // C99 6.7.8p4. All file scoped initializers need to be constant.
 if (!getLangOpts().CPlusPlus && !VDecl->isInvalidDecl())
   CheckForConstantInitializer(Init, DclT);


Index: test/Sema/dllexport-2.cpp
===
--- /dev/null
+++ test/Sema/dllexport-2.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+
+// Export const variable.
+
+__declspec(dllexport) int const j; // expected-error {{default initialization of an object of const type 'const int'}} // expected-error {{'j' must have external linkage when declared 'dllexport'}}
Index: test/Sema/dllexport-1.cpp
===
--- /dev/null
+++ test/Sema/dllexport-1.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+
+// Export const variable initialization.
+
+// expected-no-diagnostics
+__declspec(dllexport) int const x = 3;
Index: test/CodeGen/dllexport-1.c
===
--- /dev/null
+++ test/CodeGen/dllexport-1.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -emit-llvm -fms-extensions -o - %s | FileCheck %s
+
+
+// Export const variable.
+
+// CHECK: @x = dso_local dllexport constant i32 3, align 4
+// CHECK: @z = dso_local constant i32 4, align 4
+// CHECK: @y = common dso_local dllexport global i32 0, align 4
+
+__declspec(dllexport) int const x = 3;
+__declspec(dllexport) const int y;
+extern int const z = 4; // expected-warning{{'extern' variable has an initializer}}
+
+int main() {
+  int a = x + y + z;
+  return a;
+}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11367,6 +11367,16 @@
 !isTemplateInstantiation(VDecl->getTemplateSpecializationKind()))
   Diag(VDecl->getLocation(), diag::warn_extern_init);
 
+// In Microsoft C++ mode, a const variable defined in namespace scope has
+// external linkage by default if the variable is declared with
+// __declspec(dllexport).
+if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+getLangOpts().CPlusPlus &&
+VDecl->getType().isLocalConstQualified() &&
+VDecl->hasAttr() &&
+VDecl->getDefinition())
+  VDecl->setStorageClass(SC_Extern);
+
 // C99 6.7.8p4. All file scoped initializers need to be constant.
 if (!getLangOpts().CPlusPlus && !VDecl->isI

[PATCH] D45978: dllexport const variables must have external linkage.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.




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

https://reviews.llvm.org/D45978



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

[PATCH] D57080: [clang-tidy] Delete obsolete objc-property-declaration options ✂️

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57080



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

[PATCH] D58095: [clang-tidy] Make google-objc-function-naming ignore implicit functions 🙈

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: clang-tools-extra/test/clang-tidy/Inputs/Headers/stdio.h:1
+#ifndef _STDIO_H_
+#define _STDIO_H_

stephanemoore wrote:
> stephanemoore wrote:
> > I noticed that some of the other example headers don't have `#ifdef` guards 
> > at all. I decided to still include them—perhaps mostly for my own sanity. I 
> > figured that it wasn't necessary to use the full 
> > `LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_INPUTS_HEADER_STDIO_H` for this 
> > header that is mimicking a system header so I went with the shorter 
> > `_STDIO_H_` which I think better represents what actual stdio.h headers 
> > would have. Let me know if you think something else makes more sense.
> Do I need the licensing preamble on this test input header?
Yes, please add it.



Comment at: clang-tools-extra/test/clang-tidy/Inputs/Headers/stdio.h:1-9
+#ifndef _STDIO_H_
+#define _STDIO_H_
+
+// A header intended to contain C standard input and output library
+// declarations.
+
+int printf(const char *, ...);

aaron.ballman wrote:
> stephanemoore wrote:
> > stephanemoore wrote:
> > > I noticed that some of the other example headers don't have `#ifdef` 
> > > guards at all. I decided to still include them—perhaps mostly for my own 
> > > sanity. I figured that it wasn't necessary to use the full 
> > > `LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_INPUTS_HEADER_STDIO_H` for this 
> > > header that is mimicking a system header so I went with the shorter 
> > > `_STDIO_H_` which I think better represents what actual stdio.h headers 
> > > would have. Let me know if you think something else makes more sense.
> > Do I need the licensing preamble on this test input header?
> Yes, please add it.
I don't think it matters all that much for our tests; I'm fine with the way you 
have it currently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58095



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

[PATCH] D45978: dllexport const variables must have external linkage.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


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

https://reviews.llvm.org/D45978



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

[PATCH] D58120: [Builtins] Treat `bcmp` as a builtin.

[Clement Courbet via Phabricator via cfe-commits]

courbet updated this revision to Diff 186644.
courbet added a comment.
Herald added a subscriber: jdoerfert.

- Add tests for warnings.
- Fix signature for intrinsic.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58120

Files:
  include/clang/Basic/Builtins.def
  lib/AST/Decl.cpp
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/constexpr-string.cpp
  test/SemaCXX/warn-bad-memaccess.cpp

Index: test/SemaCXX/warn-bad-memaccess.cpp
===
--- test/SemaCXX/warn-bad-memaccess.cpp
+++ test/SemaCXX/warn-bad-memaccess.cpp
@@ -3,7 +3,8 @@
 extern "C" void *memset(void *, int, unsigned);
 extern "C" void *memmove(void *s1, const void *s2, unsigned n);
 extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
-extern "C" void *memcmp(void *s1, const void *s2, unsigned n);
+extern "C" int memcmp(void *s1, const void *s2, unsigned n);
+extern "C" int bcmp(void *s1, const void *s2, unsigned n);
 
 
 // Redeclare without the extern "C" to test that we still figure out that this
@@ -59,6 +60,12 @@
   memcmp(0, &x1, sizeof x1); // \
   // expected-warning{{second operand of this 'memcmp' call is a pointer to dynamic class 'X1'; vtable pointer will be compared}} \
   // expected-note {{explicitly cast the pointer to silence this warning}}
+  bcmp(&x1, 0, sizeof x1); // \
+  // expected-warning{{first operand of this 'bcmp' call is a pointer to dynamic class 'X1'; vtable pointer will be compared}} \
+  // expected-note {{explicitly cast the pointer to silence this warning}}
+  bcmp(0, &x1, sizeof x1); // \
+  // expected-warning{{second operand of this 'bcmp' call is a pointer to dynamic class 'X1'; vtable pointer will be compared}} \
+  // expected-note {{explicitly cast the pointer to silence this warning}}
 
   __builtin_memset(&x1, 0, sizeof x1); // \
   // expected-warning {{destination for this '__builtin_memset' call is a pointer to dynamic class}} \
Index: test/SemaCXX/constexpr-string.cpp
===
--- test/SemaCXX/constexpr-string.cpp
+++ test/SemaCXX/constexpr-string.cpp
@@ -1,7 +1,9 @@
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension
+// RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=gnu++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -DGNUMODE
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension
+// RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=gnu++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -DGNUMODE
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 
@@ -15,6 +17,10 @@
   extern int strncmp(const char *s1, const char *s2, size_t n);
   extern int memcmp(const void *s1, const void *s2, size_t n);
 
+#ifdef GNUMODE
+  extern int bcmp(const void *s1, const void *s2, size_t n);
+#endif
+
   extern char *strchr(const char *s, int c);
   extern void *memchr(const void *s, int c, size_t n);
 
@@ -151,6 +157,10 @@
   constexpr int a = strcmp("hello", "world"); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strcmp' cannot be used in a constant expression}}
   constexpr int b = strncmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strncmp' cannot be used in a constant expression}}
   constexpr int c = memcmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'memcmp' cannot be used in a constant expression}}
+
+#ifdef GNUMODE
+  constexpr int d = bcmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'bcmp' cannot be used in a constant expression}}
+#endif
 }
 
 namespace MultibyteElementTests {
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -9174,23 +9174,23 @@
 getContainedDynamicClass(PointeeTy, IsContained)) {
 
   unsigned OperationType = 0;
+  const bool IsCmp = BId == Builtin::BImemcmp || BId == Builtin::BIbcmp;
   // "overwritten" if we're warning about the destination for any call
   // but memcmp; otherwise a verb appropriate to the call.
-  if (ArgIdx != 0 || BId == Builtin::BImemcmp) 

[PATCH] D58120: [Builtins] Treat `bcmp` as a builtin.

[Clement Courbet via Phabricator via cfe-commits]

courbet updated this revision to Diff 186645.
courbet added a comment.

- add __builtin_bcmp constant evaluation tests.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58120

Files:
  include/clang/Basic/Builtins.def
  lib/AST/Decl.cpp
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/constexpr-string.cpp
  test/SemaCXX/warn-bad-memaccess.cpp

Index: test/SemaCXX/warn-bad-memaccess.cpp
===
--- test/SemaCXX/warn-bad-memaccess.cpp
+++ test/SemaCXX/warn-bad-memaccess.cpp
@@ -3,7 +3,8 @@
 extern "C" void *memset(void *, int, unsigned);
 extern "C" void *memmove(void *s1, const void *s2, unsigned n);
 extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
-extern "C" void *memcmp(void *s1, const void *s2, unsigned n);
+extern "C" int memcmp(void *s1, const void *s2, unsigned n);
+extern "C" int bcmp(void *s1, const void *s2, unsigned n);
 
 
 // Redeclare without the extern "C" to test that we still figure out that this
@@ -59,6 +60,12 @@
   memcmp(0, &x1, sizeof x1); // \
   // expected-warning{{second operand of this 'memcmp' call is a pointer to dynamic class 'X1'; vtable pointer will be compared}} \
   // expected-note {{explicitly cast the pointer to silence this warning}}
+  bcmp(&x1, 0, sizeof x1); // \
+  // expected-warning{{first operand of this 'bcmp' call is a pointer to dynamic class 'X1'; vtable pointer will be compared}} \
+  // expected-note {{explicitly cast the pointer to silence this warning}}
+  bcmp(0, &x1, sizeof x1); // \
+  // expected-warning{{second operand of this 'bcmp' call is a pointer to dynamic class 'X1'; vtable pointer will be compared}} \
+  // expected-note {{explicitly cast the pointer to silence this warning}}
 
   __builtin_memset(&x1, 0, sizeof x1); // \
   // expected-warning {{destination for this '__builtin_memset' call is a pointer to dynamic class}} \
Index: test/SemaCXX/constexpr-string.cpp
===
--- test/SemaCXX/constexpr-string.cpp
+++ test/SemaCXX/constexpr-string.cpp
@@ -1,7 +1,9 @@
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension
+// RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=gnu++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -DGNUMODE
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension
+// RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=gnu++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -DGNUMODE
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 
@@ -15,6 +17,10 @@
   extern int strncmp(const char *s1, const char *s2, size_t n);
   extern int memcmp(const void *s1, const void *s2, size_t n);
 
+#ifdef GNUMODE
+  extern int bcmp(const void *s1, const void *s2, size_t n);
+#endif
+
   extern char *strchr(const char *s, int c);
   extern void *memchr(const void *s, int c, size_t n);
 
@@ -101,12 +107,28 @@
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 6) == -1);
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 5) == 0);
 
+  static_assert(__builtin_bcmp("abaa", "abba", 3) != 0);
+  static_assert(__builtin_bcmp("abaa", "abba", 2) == 0);
+  static_assert(__builtin_bcmp("a\203", "a", 2) != 0);
+  static_assert(__builtin_bcmp("a\203", "a\003", 2) != 0);
+  static_assert(__builtin_bcmp(0, 0, 0) == 0);
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0banana", 100) == 0); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}}
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 100) != 0); // FIXME: Should we reject this?
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 7) != 0);
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 6) != 0);
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 5) == 0);
+
   extern struct Incomplete incomplete;
   static_assert(__builtin_memcmp(&incomplete, "", 0u) == 0);
   static_assert(__builtin_memcmp("", &incomplete, 0u) == 0);
   static_assert(__builtin_memcmp(&incomplete, "", 1u) == 42); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
   static_assert(__builtin_memcmp("", &incomplete, 1u) == 42); // expected-error {{not an integral constant}} expecte

[PATCH] D57855: [analyzer] Reimplement checker options

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.
Herald added a subscriber: jdoerfert.



Comment at: include/clang/StaticAnalyzer/Checkers/CheckerBase.td:13-14
 
+/// Describes a checker or package option type. This is important for 
validating
+/// user supplied inputs.
+class CmdLineOptionTypeEnum val> {

Might want to add a comment mentioning that additional enumerators require 
changes to the tablegen code.



Comment at: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:166
+
+PackageInfo(StringRef FullName) : FullName(FullName) {}
+  };

Make this `explicit`?



Comment at: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:248-250
+  void printCheckerWithDescList(raw_ostream &out,
+size_t maxNameChars = 30) const;
+  void printEnabledCheckerList(raw_ostream &out) const;

If we're changing these, can you fix up the parameter names for our coding 
conventions?



Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:295
+
+  SameFullName(StringRef RequestedName) : RequestedName(RequestedName) {}
+

`explicit`?



Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:306
+
+void CheckerRegistry::addDependency(StringRef fullName, StringRef dependency) {
+  auto CheckerIt = llvm::find_if(Checkers, SameCheckerName(fullName));

Naming conventions.



Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:349
+
+  PackageIt->CmdLineOptions.push_back(
+  {OptionType, OptionName, DefaultValStr, Description});

`emplace_back()` instead?



Comment at: utils/TableGen/ClangSACheckersEmitter.cpp:95
+  if (BitsInit *BI = R.getValueAsBitsInit("Type")) {
+switch(getValueFromBitsInit(BI, R)) {
+  case 0:

There should be come comments here marrying this up with the .td file.

Also, the formatting here looks somewhat off (the case statement seem to be 
indented too far).



Comment at: utils/TableGen/ClangSACheckersEmitter.cpp:171
+"#ifdef GET_PACKAGE_OPTIONS\n";
+  for (const Record *package : packages) {
+

Naming conventions.



Comment at: utils/TableGen/ClangSACheckersEmitter.cpp:253
+"#ifdef GET_CHECKER_OPTIONS\n";
+  for (const Record *checker : checkers) {
+

Naming conventions.



Comment at: utils/TableGen/ClangSACheckersEmitter.cpp:258
+
+
+std::vector CheckerOptions = checker

Can remove some of the extra vertical whitespace.


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

https://reviews.llvm.org/D57855



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

[PATCH] D58120: [Builtins] Treat `bcmp` as a builtin.

[Clement Courbet via Phabricator via cfe-commits]

courbet updated this revision to Diff 186651.
courbet added a comment.

Update tests after constness changes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58120

Files:
  include/clang/Basic/Builtins.def
  lib/AST/Decl.cpp
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaChecking.cpp
  test/Analysis/bstring.c
  test/Analysis/security-syntax-checks.m
  test/SemaCXX/constexpr-string.cpp
  test/SemaCXX/warn-bad-memaccess.cpp

Index: test/SemaCXX/warn-bad-memaccess.cpp
===
--- test/SemaCXX/warn-bad-memaccess.cpp
+++ test/SemaCXX/warn-bad-memaccess.cpp
@@ -3,7 +3,8 @@
 extern "C" void *memset(void *, int, unsigned);
 extern "C" void *memmove(void *s1, const void *s2, unsigned n);
 extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
-extern "C" void *memcmp(void *s1, const void *s2, unsigned n);
+extern "C" int memcmp(void *s1, const void *s2, unsigned n);
+extern "C" int bcmp(void *s1, const void *s2, unsigned n);
 
 
 // Redeclare without the extern "C" to test that we still figure out that this
@@ -59,6 +60,12 @@
   memcmp(0, &x1, sizeof x1); // \
   // expected-warning{{second operand of this 'memcmp' call is a pointer to dynamic class 'X1'; vtable pointer will be compared}} \
   // expected-note {{explicitly cast the pointer to silence this warning}}
+  bcmp(&x1, 0, sizeof x1); // \
+  // expected-warning{{first operand of this 'bcmp' call is a pointer to dynamic class 'X1'; vtable pointer will be compared}} \
+  // expected-note {{explicitly cast the pointer to silence this warning}}
+  bcmp(0, &x1, sizeof x1); // \
+  // expected-warning{{second operand of this 'bcmp' call is a pointer to dynamic class 'X1'; vtable pointer will be compared}} \
+  // expected-note {{explicitly cast the pointer to silence this warning}}
 
   __builtin_memset(&x1, 0, sizeof x1); // \
   // expected-warning {{destination for this '__builtin_memset' call is a pointer to dynamic class}} \
Index: test/SemaCXX/constexpr-string.cpp
===
--- test/SemaCXX/constexpr-string.cpp
+++ test/SemaCXX/constexpr-string.cpp
@@ -1,7 +1,9 @@
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension
+// RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=gnu++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -DGNUMODE
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension
+// RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=gnu++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -DGNUMODE
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 
@@ -15,6 +17,10 @@
   extern int strncmp(const char *s1, const char *s2, size_t n);
   extern int memcmp(const void *s1, const void *s2, size_t n);
 
+#ifdef GNUMODE
+  extern int bcmp(const void *s1, const void *s2, size_t n);
+#endif
+
   extern char *strchr(const char *s, int c);
   extern void *memchr(const void *s, int c, size_t n);
 
@@ -101,12 +107,28 @@
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 6) == -1);
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 5) == 0);
 
+  static_assert(__builtin_bcmp("abaa", "abba", 3) != 0);
+  static_assert(__builtin_bcmp("abaa", "abba", 2) == 0);
+  static_assert(__builtin_bcmp("a\203", "a", 2) != 0);
+  static_assert(__builtin_bcmp("a\203", "a\003", 2) != 0);
+  static_assert(__builtin_bcmp(0, 0, 0) == 0);
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0banana", 100) == 0); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}}
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 100) != 0); // FIXME: Should we reject this?
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 7) != 0);
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 6) != 0);
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 5) == 0);
+
   extern struct Incomplete incomplete;
   static_assert(__builtin_memcmp(&incomplete, "", 0u) == 0);
   static_assert(__builtin_memcmp("", &incomplete, 0u) == 0);
   static_assert(__builtin_memcmp(&incomplete, "", 1u) == 42); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
   static_assert(__builtin_memcmp("", &incomplete, 1u) == 42

[PATCH] D57855: [analyzer] Reimplement checker options

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

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

Since my changes are very invasive towards `CheckerRegistry` in general, I 
might `clang-format` the whole thing, and make the code follow the coding 
guidelines. Thanks for the review!


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

https://reviews.llvm.org/D57855



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

[PATCH] D58152: [Sema] Delay checking whether objc_designated_initializer is being applied to an init method

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5252
 const ParsedAttr &AL) {
+  auto *Ctx = D->getDeclContext();
+

Please don't use `auto` here.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7258-7267
+  // Do this check after processing D's attributes because the attribute
+  // objc_method_family can change whether the given method is in the init
+  // family, and it can be applied after objc_designated_initializer. This is a
+  // bit of a hack, but we need it to be compatible with versions of clang that
+  // processed the attribute list in the wrong order.
+  if (D->hasAttr() &&
+  cast(D)->getMethodFamily() != OMF_init) {

Do you also have to handle redeclaration merging, or is that not a thing for 
ObjC method declarations?

I'm wondering if this is better handled with the `mergeFooAttr()` pattern from 
`Sema::mergeDeclAttribute()`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58152



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

[PATCH] D58178: isRawStringLiteral doesn't check all erroneous cases

[Goran Mitrovic via Phabricator via cfe-commits]

gmit created this revision.
gmit added a reviewer: alexfh.
gmit added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

isRawStringLiteral will read from memory randomly if double quote is not found.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58178

Files:
  RawStringLiteralCheck.cpp


Index: RawStringLiteralCheck.cpp
===
--- RawStringLiteralCheck.cpp
+++ RawStringLiteralCheck.cpp
@@ -38,7 +38,8 @@
   // Already a raw string literal if R comes before ".
   const size_t QuotePos = Text.find('"');
   assert(QuotePos != StringRef::npos);
-  return (QuotePos > 0) && (Text[QuotePos - 1] == 'R');
+  return (QuotePos != StringRef::npos) && (QuotePos > 0) &&
+ (Text[QuotePos - 1] == 'R');
 }
 
 bool containsEscapedCharacters(const MatchFinder::MatchResult &Result,


Index: RawStringLiteralCheck.cpp
===
--- RawStringLiteralCheck.cpp
+++ RawStringLiteralCheck.cpp
@@ -38,7 +38,8 @@
   // Already a raw string literal if R comes before ".
   const size_t QuotePos = Text.find('"');
   assert(QuotePos != StringRef::npos);
-  return (QuotePos > 0) && (Text[QuotePos - 1] == 'R');
+  return (QuotePos != StringRef::npos) && (QuotePos > 0) &&
+ (Text[QuotePos - 1] == 'R');
 }
 
 bool containsEscapedCharacters(const MatchFinder::MatchResult &Result,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58179: [OpenCL][PR40707] Allow OpenCL C types in C++ mode

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added a reviewer: svenvh.
Herald added subscribers: ebevhan, yaxunl.

Allow all OpenCL types to be parsed in C++ mode.

I plan to enable more tests but they are currently failing for different 
reasons. This commit makes sure images can be used as they are reported in the 
bug.


https://reviews.llvm.org/D58179

Files:
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/TokenKinds.def
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseTentative.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenOpenCL/images.cl
  test/SemaOpenCL/invalid-image.cl
  test/SemaOpenCLCXX/restricted.cl

Index: test/SemaOpenCLCXX/restricted.cl
===
--- test/SemaOpenCLCXX/restricted.cl
+++ test/SemaOpenCLCXX/restricted.cl
@@ -39,25 +39,3 @@
   thread_local int y;
   // expected-error@-1 {{OpenCL C++ version 1.0 does not support the 'thread_local' storage class specifier}}
 }
-
-// Test that access qualifiers are reserved keywords.
-kernel void test_access_qualifiers() {
-  int read_only;
-  // expected-error@-1 {{'read_only' is a reserved keyword in OpenCL C++}}
-  // expected-warning@-2 {{declaration does not declare anything}}
-  int __read_only;
-  // expected-error@-1 {{'__read_only' is a reserved keyword in OpenCL C++}}
-  // expected-warning@-2 {{declaration does not declare anything}}
-  int write_only;
-  // expected-error@-1 {{'write_only' is a reserved keyword in OpenCL C++}}
-  // expected-warning@-2 {{declaration does not declare anything}}
-  int __write_only;
-  // expected-error@-1 {{'__write_only' is a reserved keyword in OpenCL C++}}
-  // expected-warning@-2 {{declaration does not declare anything}}
-  int read_write;
-  // expected-error@-1 {{'read_write' is a reserved keyword in OpenCL C++}}
-  // expected-warning@-2 {{declaration does not declare anything}}
-  int __read_write;
-  // expected-error@-1 {{'__read_write' is a reserved keyword in OpenCL C++}}
-  // expected-warning@-2 {{declaration does not declare anything}}
-}
Index: test/SemaOpenCL/invalid-image.cl
===
--- test/SemaOpenCL/invalid-image.cl
+++ test/SemaOpenCL/invalid-image.cl
@@ -1,7 +1,8 @@
+// RUN: %clang_cc1 -verify -cl-std=c++ %s
 // RUN: %clang_cc1 -verify %s
 // RUN: %clang_cc1 -verify -D=ATTR_TEST -fms-compatibility %s
 
-void test1(image1d_t *i) {} // expected-error{{pointer to type '__read_only image1d_t' is invalid in OpenCL}}
+void test1(image1d_t *i) {} // expected-error-re{{pointer to type '{{__generic __read_only|__read_only}} image1d_t' is invalid in OpenCL}}
 
 void test2(image1d_t i) {
   image1d_t ti;// expected-error{{type '__read_only image1d_t' can only be used as a function parameter}}
Index: test/CodeGenOpenCL/images.cl
===
--- test/CodeGenOpenCL/images.cl
+++ test/CodeGenOpenCL/images.cl
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 -emit-llvm -o - -cl-std=c++ | FileCheck %s
 
 __attribute__((overloadable)) void read_image(read_only image1d_t img_ro);
 __attribute__((overloadable)) void read_image(write_only image1d_t img_wo);
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6289,7 +6289,9 @@
   if (const auto *PDecl = dyn_cast(D)) {
 const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
 if (AL.getName()->getName().find("read_write") != StringRef::npos) {
-  if (S.getLangOpts().OpenCLVersion < 200 || DeclTy->isPipeType()) {
+  if ((!S.getLangOpts().OpenCLCPlusPlus &&
+   S.getLangOpts().OpenCLVersion < 200) ||
+  DeclTy->isPipeType()) {
 S.Diag(AL.getLoc(), diag::err_opencl_invalid_read_write)
 << AL << PDecl->getType() << DeclTy->isImageType();
 D->setInvalidDecl(true);
Index: lib/Parse/ParseTentative.cpp
===
--- lib/Parse/ParseTentative.cpp
+++ lib/Parse/ParseTentative.cpp
@@ -1410,11 +1410,16 @@
 // cv-qualifier
   case tok::kw_const:
   case tok::kw_volatile:
+// OpenCL address space qualifiers
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
   case tok::kw___constant:
   case tok::kw___generic:
+// OpenCL access qualifiers
+  case tok::kw___read_only:
+  case tok::kw___write_only:
+  case tok::kw___read_write:
 
 // GNU
   case tok::kw_restrict:
@@ -1600,6 +1605,8 @@
   case tok::kw___float128:
   case tok::kw_void:
   case tok::annot_decltype:
+#define GENERIC_IMAGE_TYPE(ImgType, Id) case tok::kw_##ImgType##_t:
+#include "clang/Basic/OpenCLImageTypes.def"
 if (NextToken().is(tok::l_paren))
   return TPResult::Ambiguous

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Parse/Parser.h:374
+/// This context is at the top level of a GNU statement expression.
+InStmtExpr = 0x4,
+

It's a bit strange that the previous two enumerators are "Allow" and this is 
"In". Maybe it will be less of a concern when I see the uses though...



Comment at: include/clang/Parse/Parser.h:398
+
+
+  /// Act on an expression statement that might be the last statement in a

Spurious newline.



Comment at: lib/Parse/ParseStmt.cpp:1037
 
+  ParsedStmtContext SubStmtCtx = ParsedStmtContext::Compound;
+  if (isStmtExpr)

```
ParsedStmtContext SubStmtCtx =
  ParsedStmtContext::Compound |
  (isStmtExpr ? ParsedStmtContext::InStmtExpr : 0);
```
?



Comment at: lib/Parse/ParseStmt.cpp:1053
 if (Tok.isNot(tok::kw___extension__)) {
-  R = ParseStatementOrDeclaration(Stmts, ACK_Any);
+  R = ParseStatementOrDeclaration(Stmts, SubStmtCtx, nullptr);
 } else {

You can drop the `nullptr` here.



Comment at: lib/Parse/ParseStmt.cpp:1090-1091
+R = handleExprStmt(Res, SubStmtCtx);
+if (R.isUsable())
+  R = Actions.ProcessStmtAttributes(R.get(), attrs, attrs.Range);
   }

Should this be done as part of `handleExprStmt()`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57984



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

[PATCH] D45978: dllexport const variables must have external linkage.

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:11370
 
+// In Microsoft C++ mode, a const variable defined in namespace scope has
+// external linkage by default if the variable is declared with

Even in unnamed namespaces?


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

https://reviews.llvm.org/D45978



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

[PATCH] D58178: isRawStringLiteral doesn't check all erroneous cases

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

1. test?
2. please always upload all patches with full context (`-U9`)
3. The diff is malformed. `RawStringLiteralCheck.cpp` is not on the top level 
of the repo


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58178



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

[PATCH] D45978: dllexport const variables must have external linkage.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:11370
 
+// In Microsoft C++ mode, a const variable defined in namespace scope has
+// external linkage by default if the variable is declared with

thakis wrote:
> Even in unnamed namespaces?
That would definitely be good to test.


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

https://reviews.llvm.org/D45978



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

[PATCH] D58186: Sync some doc changes ClangFormatStyleOptions.rst with doc comments in `Format.h`

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler created this revision.
rdwampler added reviewers: eugene, sylvestre.ledru, djasper.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These changes were corrected directly in ClangFormatStyleOptions.rst (llvm-svn: 
350192 and llvm-svn: 351976) but these sections can be produced automatically 
using `dump_format_style.py` so sync the corresponding doc comments in 
`Format.h` as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58186

Files:
  clang/include/clang/Format/Format.h
  clang/include/clang/Tooling/Inclusions/IncludeStyle.h


Index: clang/include/clang/Tooling/Inclusions/IncludeStyle.h
===
--- clang/include/clang/Tooling/Inclusions/IncludeStyle.h
+++ clang/include/clang/Tooling/Inclusions/IncludeStyle.h
@@ -67,7 +67,7 @@
   /// used for ordering ``#includes``.
   ///
   /// `POSIX extended
-  /// 
`_
+  /// 
`_
   /// regular expressions are supported.
   ///
   /// These regular expressions are matched against the filename of an include
@@ -79,7 +79,7 @@
   /// If none of the regular expressions match, INT_MAX is assigned as
   /// category. The main header for a source file automatically gets category 
0.
   /// so that it is generally kept at the beginning of the ``#includes``
-  /// (http://llvm.org/docs/CodingStandards.html#include-style). However, you
+  /// (https://llvm.org/docs/CodingStandards.html#include-style). However, you
   /// can also assign negative priorities if you have certain headers that
   /// always need to be first.
   ///
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1130,7 +1130,7 @@
 
   /// A vector of prefixes ordered by the desired groups for Java imports.
   ///
-  /// Each group is seperated by a newline. Static imports will also follow the
+  /// Each group is separated by a newline. Static imports will also follow the
   /// same grouping convention above all non-static imports. One group's prefix
   /// can be a subset of another - the longest prefix is always matched. Within
   /// a group, the imports are ordered lexicographically.


Index: clang/include/clang/Tooling/Inclusions/IncludeStyle.h
===
--- clang/include/clang/Tooling/Inclusions/IncludeStyle.h
+++ clang/include/clang/Tooling/Inclusions/IncludeStyle.h
@@ -67,7 +67,7 @@
   /// used for ordering ``#includes``.
   ///
   /// `POSIX extended
-  /// `_
+  /// `_
   /// regular expressions are supported.
   ///
   /// These regular expressions are matched against the filename of an include
@@ -79,7 +79,7 @@
   /// If none of the regular expressions match, INT_MAX is assigned as
   /// category. The main header for a source file automatically gets category 0.
   /// so that it is generally kept at the beginning of the ``#includes``
-  /// (http://llvm.org/docs/CodingStandards.html#include-style). However, you
+  /// (https://llvm.org/docs/CodingStandards.html#include-style). However, you
   /// can also assign negative priorities if you have certain headers that
   /// always need to be first.
   ///
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1130,7 +1130,7 @@
 
   /// A vector of prefixes ordered by the desired groups for Java imports.
   ///
-  /// Each group is seperated by a newline. Static imports will also follow the
+  /// Each group is separated by a newline. Static imports will also follow the
   /// same grouping convention above all non-static imports. One group's prefix
   /// can be a subset of another - the longest prefix is always matched. Within
   /// a group, the imports are ordered lexicographically.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58091: Customize warnings for missing built-in type

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: rsmith.
aaron.ballman added a comment.

If I'm following along properly, it sounds like we want to disable this warning 
largely because it can appear in header files attempting to declare the 
functions in question -- but I wonder why those diagnostics are happening in 
the first place. It seems like the warning is still useful when it triggers 
outside of that situation, no?




Comment at: clang/lib/Sema/SemaDecl.cpp:1974
+
+// Generally we emit a warning that the declaration requires the 
appropriate
+// header.

Generally -> Generally,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58091



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

[PATCH] D58185: [clangd] Handle unresolved scope specifier when fixing includes.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, jdoerfert, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.
ioeric updated this revision to Diff 186674.
ioeric added a comment.
ioeric edited the summary of this revision.

- Remove unintended change.


In the following examples, "clangd" is unresolved, and the fixer will try to fix
include for `clang::clangd`; however, clang::clangd::X is usually intended. So
when handling a qualifier that is unresolved, we change the unresolved name and
scopes so that the fixer will fix "clang::clangd::X" in the following example.

namespace clang {
clangd::X
~~
}
// or
clang::clangd::X
   ~~


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58185

Files:
  clangd/IncludeFixer.cpp
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -20,6 +20,7 @@
 namespace clangd {
 namespace {
 
+using testing::_;
 using testing::ElementsAre;
 using testing::Field;
 using testing::IsEmpty;
@@ -369,6 +370,8 @@
 $insert[[]]namespace ns {
 void foo() {
   $unqualified1[[X]] x;
+  // No fix if the unresolved type is used as qualifier. (ns::)X::Nested will be
+  // considered the unresolved type.
   $unqualified2[[X]]::Nested n;
 }
 }
@@ -391,10 +394,7 @@
   AllOf(Diag(Test.range("unqualified1"), "unknown type name 'X'"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),
-  AllOf(Diag(Test.range("unqualified2"),
- "use of undeclared identifier 'X'"),
-WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
-"Add include \"x.h\" for symbol ns::X"))),
+  Diag(Test.range("unqualified2"), "use of undeclared identifier 'X'"),
   AllOf(Diag(Test.range("qualified1"),
  "no type named 'X' in namespace 'ns'"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
@@ -449,6 +449,68 @@
   UnorderedElementsAre(Diag(Test.range(), "no member named 'xy' in 'X'")));
 }
 
+TEST(IncludeFixerTest, UnresolvedNameAsQualifier) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace ns {
+}
+void g() {  ns::$[[scope]]::X_Y();  }
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  SymbolWithHeader{"ns::scope::X_Y", "unittest:///x.h", "\"x.h\""});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Test.range(), "no member named 'scope' in namespace 'ns'"),
+  WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol ns::scope::X_Y");
+}
+
+TEST(IncludeFixerTest, UnresolvedQualifierWithSemaCorrection) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace clang {
+void f() {
+  // "clangd::" will be corrected to "clang::" by Sema.
+  $q1[[clangd]]::$x[[X]] x;
+  $q2[[clangd]]::$ns[[ns]]::Y y;
+}
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"clang::clangd::X", "unittest:///x.h", "\"x.h\""},
+   SymbolWithHeader{"clang::clangd::ns::Y", "unittest:///y.h", "\"y.h\""}});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  AllOf(
+  Diag(Test.range("q1"), "use of undeclared identifier 'clangd'; "
+ "did you mean 'clang'?"),
+  WithFix(_, // change clangd to clang
+  Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol clang::clangd::X"))),
+  AllOf(
+  Diag(Test.range("x"), "no type named 'X' in namespace 'clang'"),
+  WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol clang::clangd::X"))),
+  AllOf(
+  Diag(Test.range("q2"), "use of undeclared identifier 'clangd'; "
+ "did you mean 'clang'?"),
+  WithFix(
+  _, // change clangd to clangd
+  Fix(Test.range("insert"), "#include \"y.h\"\n",
+  "Add include \"y.h\" for symbol clang::clangd::ns::Y"))),
+  AllOf(Diag(Test.range("ns"),
+ "no member named 'ns' in namespace 'clang'"),
+WithFix(Fix(
+Test.range("insert"), "#include \"y.h\"\n",
+"Add include \"y.h\" for symbol clang::clangd::ns::Y");
+}
+
 } // namespace
 } // namespace clang

[PATCH] D58185: [clangd] Handle unresolved scope specifier when fixing includes.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 186674.
ioeric added a comment.

- Remove unintended change.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58185

Files:
  clangd/IncludeFixer.cpp
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -20,6 +20,7 @@
 namespace clangd {
 namespace {
 
+using testing::_;
 using testing::ElementsAre;
 using testing::Field;
 using testing::IsEmpty;
@@ -369,6 +370,8 @@
 $insert[[]]namespace ns {
 void foo() {
   $unqualified1[[X]] x;
+  // No fix if the unresolved type is used as qualifier. (ns::)X::Nested will be
+  // considered the unresolved type.
   $unqualified2[[X]]::Nested n;
 }
 }
@@ -391,10 +394,7 @@
   AllOf(Diag(Test.range("unqualified1"), "unknown type name 'X'"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),
-  AllOf(Diag(Test.range("unqualified2"),
- "use of undeclared identifier 'X'"),
-WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
-"Add include \"x.h\" for symbol ns::X"))),
+  Diag(Test.range("unqualified2"), "use of undeclared identifier 'X'"),
   AllOf(Diag(Test.range("qualified1"),
  "no type named 'X' in namespace 'ns'"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
@@ -449,6 +449,68 @@
   UnorderedElementsAre(Diag(Test.range(), "no member named 'xy' in 'X'")));
 }
 
+TEST(IncludeFixerTest, UnresolvedNameAsQualifier) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace ns {
+}
+void g() {  ns::$[[scope]]::X_Y();  }
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  SymbolWithHeader{"ns::scope::X_Y", "unittest:///x.h", "\"x.h\""});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Test.range(), "no member named 'scope' in namespace 'ns'"),
+  WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol ns::scope::X_Y");
+}
+
+TEST(IncludeFixerTest, UnresolvedQualifierWithSemaCorrection) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace clang {
+void f() {
+  // "clangd::" will be corrected to "clang::" by Sema.
+  $q1[[clangd]]::$x[[X]] x;
+  $q2[[clangd]]::$ns[[ns]]::Y y;
+}
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"clang::clangd::X", "unittest:///x.h", "\"x.h\""},
+   SymbolWithHeader{"clang::clangd::ns::Y", "unittest:///y.h", "\"y.h\""}});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  AllOf(
+  Diag(Test.range("q1"), "use of undeclared identifier 'clangd'; "
+ "did you mean 'clang'?"),
+  WithFix(_, // change clangd to clang
+  Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol clang::clangd::X"))),
+  AllOf(
+  Diag(Test.range("x"), "no type named 'X' in namespace 'clang'"),
+  WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol clang::clangd::X"))),
+  AllOf(
+  Diag(Test.range("q2"), "use of undeclared identifier 'clangd'; "
+ "did you mean 'clang'?"),
+  WithFix(
+  _, // change clangd to clangd
+  Fix(Test.range("insert"), "#include \"y.h\"\n",
+  "Add include \"y.h\" for symbol clang::clangd::ns::Y"))),
+  AllOf(Diag(Test.range("ns"),
+ "no member named 'ns' in namespace 'clang'"),
+WithFix(Fix(
+Test.range("insert"), "#include \"y.h\"\n",
+"Add include \"y.h\" for symbol clang::clangd::ns::Y");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/IncludeFixer.cpp
===
--- clangd/IncludeFixer.cpp
+++ clangd/IncludeFixer.cpp
@@ -19,6 +19,7 @@
 #include "clang/AST/Type.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticSema.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Scope.h"
@@ -27,6 +28,8 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringR

r353952 - [HIP] Handle compile -m options and propagate into LLC

[Aaron Enye Shi via cfe-commits]

Author: aaronenyeshi
Date: Wed Feb 13 08:12:16 2019
New Revision: 353952

URL: http://llvm.org/viewvc/llvm-project?rev=353952&view=rev
Log:
[HIP] Handle compile -m options and propagate into LLC

Allow the compile options for -m such as -mxnack/-mno-xnack, 
-msram-ecc/-mno-sram-ecc, -mcode-object-v3/-mno-code-object-v3 to propagate 
into LLC args. Fix an issue where -mattr was pushed even when it was empty.

Also add lit tests to verify features are properly passed.

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

Reviewers: yaxunl, kzhuravl


Added:
cfe/trunk/test/Driver/hip-toolchain-features.hip
Modified:
cfe/trunk/lib/Driver/ToolChains/HIP.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/HIP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/HIP.cpp?rev=353952&r1=353951&r2=353952&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/HIP.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/HIP.cpp Wed Feb 13 08:12:16 2019
@@ -159,8 +159,26 @@ const char *AMDGCN::Linker::constructLlc
 llvm::StringRef OutputFilePrefix, const char *InputFileName) const {
   // Construct llc command.
   ArgStringList LlcArgs{InputFileName, "-mtriple=amdgcn-amd-amdhsa",
-"-filetype=obj", "-mattr=-code-object-v3",
-Args.MakeArgString("-mcpu=" + SubArchName), "-o"};
+"-filetype=obj",
+Args.MakeArgString("-mcpu=" + SubArchName)};
+
+  // Extract all the -m options
+  std::vector Features;
+  handleTargetFeaturesGroup(
+Args, Features, options::OPT_m_amdgpu_Features_Group);
+
+  // Add features to mattr such as code-object-v3 and xnack
+  std::string MAttrString = "-mattr=";
+  for(auto OneFeature : Features) {
+MAttrString.append(Args.MakeArgString(OneFeature));
+if (OneFeature != Features.back())
+  MAttrString.append(",");
+  }
+  if(!Features.empty())
+LlcArgs.push_back(Args.MakeArgString(MAttrString));
+
+  // Add output filename
+  LlcArgs.push_back("-o");
   std::string LlcOutputFileName =
   C.getDriver().GetTemporaryPath(OutputFilePrefix, "o");
   const char *LlcOutputFile =

Added: cfe/trunk/test/Driver/hip-toolchain-features.hip
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hip-toolchain-features.hip?rev=353952&view=auto
==
--- cfe/trunk/test/Driver/hip-toolchain-features.hip (added)
+++ cfe/trunk/test/Driver/hip-toolchain-features.hip Wed Feb 13 08:12:16 2019
@@ -0,0 +1,48 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mcode-object-v3 2>&1 | FileCheck %s -check-prefix=COV3
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mno-code-object-v3 2>&1 | FileCheck %s -check-prefix=NOCOV3
+
+// COV3: {{.*}}clang{{.*}}"-target-feature" "+code-object-v3"
+// NOCOV3: {{.*}}clang{{.*}}"-target-feature" "-code-object-v3"
+
+
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mxnack 2>&1 | FileCheck %s -check-prefix=XNACK
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mno-xnack 2>&1 | FileCheck %s -check-prefix=NOXNACK
+
+// XNACK: {{.*}}clang{{.*}}"-target-feature" "+xnack"
+// NOXNACK: {{.*}}clang{{.*}}"-target-feature" "-xnack"
+
+
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -msram-ecc 2>&1 | FileCheck %s -check-prefix=SRAM
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mno-sram-ecc 2>&1 | FileCheck %s -check-prefix=NOSRAM
+
+// SRAM: {{.*}}clang{{.*}}"-target-feature" "+sram-ecc"
+// NOSRAM: {{.*}}clang{{.*}}"-target-feature" "-sram-ecc"
+
+
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mcode-object-v3 -mxnack -msram-ecc \
+// RUN:   2>&1 | FileCheck %s -check-prefix=ALL3
+// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
+// RUN:   -mno-code-object-v3 -mno-xnack -mno-sram-ecc \
+// RUN:   2>&1 | FileCheck %s -check-prefix=NOALL3
+
+// ALL3: {{.*}}clang{{.*}}"-target-feature" "+code-object-v3" 
"-target-feature" "+xnack" "-target-feature" "+sram-ecc"
+// NOALL3: {{.*}}clang{{.*}}"-target-feature" "-code-object-v3" 
"-target-feature" "-xnack" "-target-feature" "-sram-ecc"


_

[PATCH] D58178: isRawStringLiteral doesn't check all erroneous cases

[Goran Mitrovic via Phabricator via cfe-commits]

gmit updated this revision to Diff 186675.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58178

Files:
  RawStringLiteralCheck.cpp
  
clang-tools-extra/tags/RELEASE_800/rc2/clang-tidy/modernize/RawStringLiteralCheck.cpp


Index: 
clang-tools-extra/tags/RELEASE_800/rc2/clang-tidy/modernize/RawStringLiteralCheck.cpp
===
--- 
clang-tools-extra/tags/RELEASE_800/rc2/clang-tidy/modernize/RawStringLiteralCheck.cpp
+++ 
clang-tools-extra/tags/RELEASE_800/rc2/clang-tidy/modernize/RawStringLiteralCheck.cpp
@@ -38,7 +38,8 @@
   // Already a raw string literal if R comes before ".
   const size_t QuotePos = Text.find('"');
   assert(QuotePos != StringRef::npos);
-  return (QuotePos > 0) && (Text[QuotePos - 1] == 'R');
+  return (QuotePos != StringRef::npos) && (QuotePos > 0) &&
+ (Text[QuotePos - 1] == 'R');
 }
 
 bool containsEscapedCharacters(const MatchFinder::MatchResult &Result,


Index: clang-tools-extra/tags/RELEASE_800/rc2/clang-tidy/modernize/RawStringLiteralCheck.cpp
===
--- clang-tools-extra/tags/RELEASE_800/rc2/clang-tidy/modernize/RawStringLiteralCheck.cpp
+++ clang-tools-extra/tags/RELEASE_800/rc2/clang-tidy/modernize/RawStringLiteralCheck.cpp
@@ -38,7 +38,8 @@
   // Already a raw string literal if R comes before ".
   const size_t QuotePos = Text.find('"');
   assert(QuotePos != StringRef::npos);
-  return (QuotePos > 0) && (Text[QuotePos - 1] == 'R');
+  return (QuotePos != StringRef::npos) && (QuotePos > 0) &&
+ (Text[QuotePos - 1] == 'R');
 }
 
 bool containsEscapedCharacters(const MatchFinder::MatchResult &Result,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58178: isRawStringLiteral doesn't check all erroneous cases

[Goran Mitrovic via Phabricator via cfe-commits]

gmit added a comment.

Sorry, this is my first LLVM commit, so I don't really know the procedure.

There is no test as it cannot be reproduced. The crash caused by this was 
spotted in one of our customer's crash dumps. The added condition should be 
obvious as it is tested in the assert just above.

Could you advise how to make a diff you'd like with TortoiseSVN? I don't seem 
to be able to add additional parameters. I have attached a new one with full 
path to the file.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58178



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

[PATCH] D58178: [clang-tidy] RawStringLiteralCheck: isRawStringLiteral doesn't check all erroneous cases

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D58178#1396388 , @gmit wrote:

> The added condition should be obvious as it is tested in the assert just 
> above.


Well, then the fix is not correct. Because you won't get to that `return`,
since the `assert()` just above would have failed already..
This really needs a test.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58178



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

[PATCH] D58091: Customize warnings for missing built-in type

[Brian Cain via Phabricator via cfe-commits]

bcain added a comment.

I reported PR40692.  I just tried this patch on our local build where we saw 
the failure on an RTOS implementing pthreads.  Unfortunately with this patch I 
encountered an (unrelated) assertion.  So this fix was inconclusive for me (for 
now).  I will follow up but if this fix makes sense then you don't need to wait 
for my test results.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58091



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

[PATCH] D58091: Customize warnings for missing built-in type

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D58091#1396382 , @aaron.ballman 
wrote:

> If I'm following along properly, it sounds like we want to disable this 
> warning largely because it can appear in header files attempting to declare 
> the functions in question.


That is the situation that exposed the problem, yes.

> - but I wonder why those diagnostics are happening in the first place. It 
> seems like the warning is still useful when it triggers outside of that 
> situation, no?

The underlying conceptual problem, which I didn't know when I added 
`GE_Missing_type`, is that this has _nothing_ to do with the location of the 
declaration. We say, include the header X.h, if we were not able to build a 
type for recognized built-in Y that should be declared in X.h. However, we 
should report _why_ we could not build the type instead. For built-ins we do 
not have a type on record (`GE_Missing_type`), this is always, so no warning 
for now. For the ones that we only fail to build a type because some 
requirement is missing, we should report that, at least when we are in the 
respective header. I don't have a perfect solution of what to do actually.

I could check if the declaration is (probably) in the respective header so we 
can switch between warnings?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58091



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

[PATCH] D58091: Customize warnings for missing built-in type

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D58091#1396393 , @bcain wrote:

> I reported PR40692.  I just tried this patch on our local build where we saw 
> the failure on an RTOS implementing pthreads.  Unfortunately with this patch 
> I encountered an (unrelated) assertion.  So this fix was inconclusive for me 
> (for now).  I will follow up but if this fix makes sense then you don't need 
> to wait for my test results.


Are you sure it is unrelated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58091



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

[PATCH] D54978: Move the SMT API to LLVM

[Brian Rzycki via Phabricator via cfe-commits]

brzycki added a comment.

> perhaps something like this:
> 
>   if(Z3_INCLUDE_DIR AND EXISTS "${Z3_INCLUDE_DIR }/z3_version.h")
> file(STRINGS "${Z3_INCLUDE_DIR }/z3_version.h" z3_version_str REGEX 
> "^#define[\t ]+Z3_FULL_VERSION[\t ]+\".*\"")
>   
> string(REGEX REPLACE "^.*Z3_FULL_VERSION[\t ]+\"Z3 ([0-9\.]+)\".*$" "\\1" 
> Z3_VERSION_STRING "${z3_version_str}")
> unset(z3_version_str)
>   endif()

That is almost exactly what I was thinking about this morning. I'd prefer the 
following since it's more robust for older versions:

  if(Z3_INCLUDE_DIR AND EXISTS "${Z3_INCLUDE_DIR }/z3_version.h")
# For 4.8.0 or newer
file(STRINGS "${Z3_INCLUDE_DIR }/z3_version.h" z3_version_str REGEX 
"^#define[\t ]+Z3_FULL_VERSION[\t ]+\".*\"")
  
string(REGEX REPLACE "^.*Z3_FULL_VERSION[\t ]+\"Z3 ([0-9\.]+)\".*$" "\\1" 
Z3_VERSION_STRING "${z3_version_str}")
unset(z3_version_str)
  elseif(Z3_INCLUDE_DIR AND EXISTS "${Z3_INCLUDE_DIR }/version.h")
file(STRINGS "${Z3_INCLUDE_DIR }/version.h" z3_version_str REGEX 
"^#define[\t ]+Z3_FULL_VERSION[\t ]+\".*\"")
  
string(REGEX REPLACE "^.*Z3_FULL_VERSION[\t ]+\"Z3 ([0-9\.]+)\".*$" "\\1" 
Z3_VERSION_STRING "${z3_version_str}")
unset(z3_version_str)
  else()
message(WARNING "Unable to locate the Z3 version.h header. Setting the 
found version to 0.0.0.")
set(Z3_VERSION_STRING "0.0.0")
  endif()


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D58060: Fix diagnostic for addr spaces in static_cast

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

So static_cast permits conversions from AS1 to AS2 where that conversion is 
implicitly allowed, and the new addrspace_cast would permit conversions from 
AS1 to AS2 where it is explicitly allowed. That seems like it fits in rather 
well with the idea in D57464  regarding 
support for specifying permitted AS conversions in target.

How about nested pointers, such as `__X int * *` -> `__Y int * *` or `__X int * 
__Y *` -> `int * __Y *`? static_cast has some ruleset for how to deal with 
qualifiers in nested pointers, I think, but I'm not sure how the rules for ASes 
should be here.

There's also C-style casts. Will addrspace_cast be added to the C-style cast 
rules somewhere?

(I should probably have commented on the original RFC)


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

https://reviews.llvm.org/D58060



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:979
+  if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+return false;
+  if (GlobalSize <= SizeLimit)

glider wrote:
> jfb wrote:
> > The general 64-byte heuristic is fine with me. It's just a bit weird to 
> > special-case `-O0`, but we do it elsewhere too (and it keeps the current 
> > status-quo of letting the backend decide what to do). From that perspective 
> > I'm fine with it.
> > 
> > @rjmccall do you have a strong preference either way?
> > 
> > One small nit: can you use `ByteSize` instead of just `Size`? Makes it 
> > easier to figure out what's going on in code IMO.
> I don't have a strong opinion on variable naming, but this:
> ```
>  974 static bool shouldSplitStructStore(CodeGenModule &CGM,
>  975uint64_t GlobalByteSize) {
>  976   // Don't break structures that occupy more than one cacheline.
>  977   uint64_t ByteSizeLimit = 64;
>  978   if (CGM.getCodeGenOpts().OptimizationLevel == 0)
>  979 return false;
>  980   if (GlobalByteSize <= ByteSizeLimit)
>  981 return true;
>  982   return false;
> ```
> looks a bit more heavyweight than the previous function, in which we also 
> have `GlobalSize` and `SizeLimit`.
What are the cases we actually want to make sure we emit as separate stores?  
Basically just really small types that happen to be wrapped up in several 
layers of struct for abstraction purposes, right?  Maybe this heuristic should 
primarily consider element counts (and types) and use overall sizes at the top 
level.


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

https://reviews.llvm.org/D57898



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

[PATCH] D58178: [clang-tidy] RawStringLiteralCheck: isRawStringLiteral doesn't check all erroneous cases

[Goran Mitrovic via Phabricator via cfe-commits]

gmit added a comment.

? I'm sorry, but I disagree.

Assert draws attention in the debug build only.

In the release build asserts are not evaluated at all and the condition needs 
to be checked in the code that executes (if it makes sense and in this case it 
does as it causes reading out of string bounds).

I don't have a test as I don't have an input file that causes this behaviour.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58178



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

[PATCH] D58091: Customize warnings for missing built-in type

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D58091#1396397 , @jdoerfert wrote:

> In D58091#1396382 , @aaron.ballman 
> wrote:
>
> > - but I wonder why those diagnostics are happening in the first place. It 
> > seems like the warning is still useful when it triggers outside of that 
> > situation, no?
>
>
> The underlying conceptual problem, which I didn't know when I added 
> `GE_Missing_type`, is that this has _nothing_ to do with the location of the 
> declaration. We say, include the header X.h, if we were not able to build a 
> type for recognized built-in Y that should be declared in X.h. However, we 
> should report _why_ we could not build the type instead. For built-ins we do 
> not have a type on record (`GE_Missing_type`), this is always, so no warning 
> for now. For the ones that we only fail to build a type because some 
> requirement is missing, we should report that, at least when we are in the 
> respective header. I don't have a perfect solution of what to do actually.
>
> I could check if the declaration is (probably) in the respective header so we 
> can switch between warnings?


That's kind of what I was wondering, but I deal with builtins so infrequently 
that my expectations may be wrong here. If a user declares a builtin with a 
conflicting type outside of a header file, that seems like we'd want to warn 
the user about right? But this seems to remove that warning, at least in the 
case of test/Sema/implicit-builtin-decl.c:71. Or do I misunderstand the 
situation causing the warning to trigger?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58091



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D57898#1395953 , @glider wrote:

> ... which happily skips the padding.


I don't think padding is an issue right now. It's valid to either initialize it 
or not. It is slightly unfortunate to lose the information about padding (which 
the struct approach keeps). One alternative could be to store a struct when it 
has padding (i.e. not two stores of char+int, but a single store struct of 
char+int). From what I've seen the optimization pipeline doesn't do as well 
with that type of stores.

You might instead just choose to not do anything in this patch if the struct 
has padding. We can do something later.

FWIW I intend to write a separate patch which lets you opt in to padding always 
being set. I'll be touching this code anyways.

> It occurs to me that we need to properly handle the padding in `patternFor` 
> before we'll be able to split the structures.

patternFor handles padding already: it treats it as any value, and chooses 
something adjacent.


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:979
+  if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+return false;
+  if (GlobalSize <= SizeLimit)

rjmccall wrote:
> glider wrote:
> > jfb wrote:
> > > The general 64-byte heuristic is fine with me. It's just a bit weird to 
> > > special-case `-O0`, but we do it elsewhere too (and it keeps the current 
> > > status-quo of letting the backend decide what to do). From that 
> > > perspective I'm fine with it.
> > > 
> > > @rjmccall do you have a strong preference either way?
> > > 
> > > One small nit: can you use `ByteSize` instead of just `Size`? Makes it 
> > > easier to figure out what's going on in code IMO.
> > I don't have a strong opinion on variable naming, but this:
> > ```
> >  974 static bool shouldSplitStructStore(CodeGenModule &CGM,
> >  975uint64_t GlobalByteSize) {
> >  976   // Don't break structures that occupy more than one cacheline.
> >  977   uint64_t ByteSizeLimit = 64;
> >  978   if (CGM.getCodeGenOpts().OptimizationLevel == 0)
> >  979 return false;
> >  980   if (GlobalByteSize <= ByteSizeLimit)
> >  981 return true;
> >  982   return false;
> > ```
> > looks a bit more heavyweight than the previous function, in which we also 
> > have `GlobalSize` and `SizeLimit`.
> What are the cases we actually want to make sure we emit as separate stores?  
> Basically just really small types that happen to be wrapped up in several 
> layers of struct for abstraction purposes, right?  Maybe this heuristic 
> should primarily consider element counts (and types) and use overall sizes at 
> the top level.
Yes, small types. 


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

FWIW I think I've almost finished handling the padding: 
https://reviews.llvm.org/D58188
I haven't checked whether it works correctly with padding in custom 
initializers (like `struct s local = {1, 2}`), but TEST_UNINIT and TEST_BRACES 
are covered already.


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

https://reviews.llvm.org/D57898



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

[PATCH] D58189: [clang][Index] Fix usage of IndexImplicitInstantiation

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
Herald added subscribers: cfe-commits, jdoerfert, arphaman.
Herald added a project: clang.

Indexing context was skipping explicit template instantiations as well.
This patch makes sure it only skips implicit ones.


Repository:
  rC Clang

https://reviews.llvm.org/D58189

Files:
  lib/Index/IndexTypeSourceInfo.cpp
  test/Index/Core/index-source.cpp
  test/Index/index-refs.cpp
  unittests/Index/IndexTests.cpp

Index: unittests/Index/IndexTests.cpp
===
--- unittests/Index/IndexTests.cpp
+++ unittests/Index/IndexTests.cpp
@@ -7,7 +7,10 @@
 //===--===//
 
 #include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Index/IndexDataConsumer.h"
@@ -24,26 +27,60 @@
 namespace clang {
 namespace index {
 
+struct Position {
+  size_t Line;
+  size_t Column;
+
+  static Position fromSourceLocation(SourceLocation Loc,
+ const SourceManager &SM) {
+FileID FID;
+unsigned Offset;
+std::tie(FID, Offset) = SM.getDecomposedSpellingLoc(Loc);
+Position P;
+P.Line = static_cast(SM.getLineNumber(FID, Offset)) - 1;
+P.Column = SM.getColumnNumber(FID, Offset) - 1;
+return P;
+  }
+
+  bool operator==(const Position &Other) const {
+return std::tie(Line, Column) == std::tie(Other.Line, Other.Column);
+  }
+};
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Position &Pos) {
+  return OS << Pos.Line << ':' << Pos.Column;
+}
+
 struct TestSymbol {
   std::string QName;
+  Position WrittenPos;
+  Position DeclPos;
   // FIXME: add more information.
 };
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const TestSymbol &S) {
-  return OS << S.QName;
+  return OS << S.QName << '[' << S.WrittenPos << ']';
 }
 
 namespace {
 class Indexer : public IndexDataConsumer {
 public:
+  void initialize(ASTContext &Ctx) override {
+AST = &Ctx;
+IndexDataConsumer::initialize(Ctx);
+  }
+
   bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
-   ArrayRef, SourceLocation,
+   ArrayRef, SourceLocation Loc,
ASTNodeInfo) override {
 const auto *ND = llvm::dyn_cast(D);
 if (!ND)
   return true;
 TestSymbol S;
 S.QName = ND->getQualifiedNameAsString();
+S.WrittenPos = Position::fromSourceLocation(Loc, AST->getSourceManager());
+S.DeclPos =
+Position::fromSourceLocation(D->getLocation(), AST->getSourceManager());
 Symbols.push_back(std::move(S));
 return true;
   }
@@ -57,6 +94,7 @@
   }
 
   std::vector Symbols;
+  const ASTContext *AST;
 };
 
 class IndexAction : public ASTFrontendAction {
@@ -93,11 +131,14 @@
   IndexingOptions Opts;
 };
 
+using testing::AllOf;
 using testing::Contains;
 using testing::Not;
 using testing::UnorderedElementsAre;
 
 MATCHER_P(QName, Name, "") { return arg.QName == Name; }
+MATCHER_P(WrittenAt, Pos, "") { return arg.WrittenPos == Pos; }
+MATCHER_P(DeclAt, Pos, "") { return arg.DeclPos == Pos; }
 
 TEST(IndexTest, Simple) {
   auto Index = std::make_shared();
@@ -134,6 +175,44 @@
   EXPECT_THAT(Index->Symbols, Not(Contains(QName("bar";
 }
 
+TEST(IndexTest, IndexExplicitTemplateInstantiation) {
+  std::string Code = R"cpp(
+template 
+struct Foo { void bar() {} };
+template <>
+struct Foo { void bar() {} };
+void foo() {
+  Foo abc;
+  Foo b;
+}
+  )cpp";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols,
+  Contains(AllOf(QName("Foo"), WrittenAt(Position({7, 6})),
+ DeclAt(Position({4, 11});
+}
+
+TEST(IndexTest, IndexTemplateInstantiationPartial) {
+  std::string Code = R"cpp(
+template 
+struct Foo { void bar() {} };
+template 
+struct Foo { void bar() {} };
+void foo() {
+  Foo abc;
+  Foo b;
+}
+  )cpp";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols,
+  Contains(AllOf(QName("Foo"), WrittenAt(Position({7, 6})),
+ DeclAt(Position({4, 11});
+}
+
 } // namespace
 } // namespace index
 } // namespace clang
Index: test/Index/index-refs.cpp
===
--- test/Index/index-refs.cpp
+++ test/Index/index-refs.cpp
@@ -117,7 +117,7 @@
 /* when indexing implicit instantiations
   [indexEntityReference]: kind: struct-template-spec | name: TS | USR: c:@S@TS>#I | {{.*}} | loc: 55:3
 */
-// CHECK-NEXT: [indexEn

[PATCH] D58122: Restore Check for Unreachable Exit Block in -Winfinite-recursion

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D58122



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

[PATCH] D58190: [clangd] Add tests for template specializations

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric, 
ilya-biryukov.
Herald added a project: clang.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58190

Files:
  unittests/clangd/XRefsTests.cpp


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -350,6 +350,30 @@
  FF();
  void f() { T^est a; }
   )cpp",
+
+  R"cpp(
+template 
+struct Foo { void bar() {} };
+
+template <>
+struct [[Foo]] { void bar() {} };
+
+void foo() {
+  Foo abc;
+  Fo^o b;
+}
+  )cpp",
+
+  R"cpp(
+template 
+struct [[Foo]] { void bar() {} };
+template <>
+struct Foo { void bar() {} };
+void foo() {
+  Fo^o abc;
+  Foo b;
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -350,6 +350,30 @@
  FF();
  void f() { T^est a; }
   )cpp",
+
+  R"cpp(
+template 
+struct Foo { void bar() {} };
+
+template <>
+struct [[Foo]] { void bar() {} };
+
+void foo() {
+  Foo abc;
+  Fo^o b;
+}
+  )cpp",
+
+  R"cpp(
+template 
+struct [[Foo]] { void bar() {} };
+template <>
+struct Foo { void bar() {} };
+void foo() {
+  Fo^o abc;
+  Foo b;
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r353957 - [AArch64] Support reserving arbitrary general purpose registers

[Petr Hosek via cfe-commits]

Author: phosek
Date: Wed Feb 13 09:28:47 2019
New Revision: 353957

URL: http://llvm.org/viewvc/llvm-project?rev=353957&view=rev
Log:
[AArch64] Support reserving arbitrary general purpose registers

This is a follow up to D48580 and D48581 which allows reserving
arbitrary general purpose registers with the exception of registers
with special purpose (X8, X16-X18, X29, X30) and registers used by LLVM
(X0, X19). This change also generalizes some of the existing logic to
rely entirely on values generated from tablegen.

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
cfe/trunk/test/Driver/aarch64-fixed-x-register.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=353957&r1=353956&r2=353957&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed Feb 13 09:28:47 2019
@@ -2134,7 +2134,7 @@ def mfix_cortex_a53_835769 : Flag<["-"],
 def mno_fix_cortex_a53_835769 : Flag<["-"], "mno-fix-cortex-a53-835769">,
   Group,
   HelpText<"Don't workaround Cortex-A53 erratum 835769 (AArch64 only)">;
-foreach i = {1-7,18,20} in
+foreach i = {1-7,9-15,18,20-28} in
   def ffixed_x#i : Flag<["-"], "ffixed-x"#i>, Group,
 HelpText<"Reserve the "#i#" register (AArch64 only)">;
 

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp?rev=353957&r1=353956&r2=353957&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp Wed Feb 13 09:28:47 2019
@@ -334,12 +334,54 @@ fp16_fml_fallthrough:
   if (Args.hasArg(options::OPT_ffixed_x7))
 Features.push_back("+reserve-x7");
 
+  if (Args.hasArg(options::OPT_ffixed_x9))
+Features.push_back("+reserve-x9");
+
+  if (Args.hasArg(options::OPT_ffixed_x10))
+Features.push_back("+reserve-x10");
+
+  if (Args.hasArg(options::OPT_ffixed_x11))
+Features.push_back("+reserve-x11");
+
+  if (Args.hasArg(options::OPT_ffixed_x12))
+Features.push_back("+reserve-x12");
+
+  if (Args.hasArg(options::OPT_ffixed_x13))
+Features.push_back("+reserve-x13");
+
+  if (Args.hasArg(options::OPT_ffixed_x14))
+Features.push_back("+reserve-x14");
+
+  if (Args.hasArg(options::OPT_ffixed_x15))
+Features.push_back("+reserve-x15");
+
   if (Args.hasArg(options::OPT_ffixed_x18))
 Features.push_back("+reserve-x18");
 
   if (Args.hasArg(options::OPT_ffixed_x20))
 Features.push_back("+reserve-x20");
 
+  if (Args.hasArg(options::OPT_ffixed_x21))
+Features.push_back("+reserve-x21");
+
+  if (Args.hasArg(options::OPT_ffixed_x22))
+Features.push_back("+reserve-x22");
+
+  if (Args.hasArg(options::OPT_ffixed_x23))
+Features.push_back("+reserve-x23");
+
+  if (Args.hasArg(options::OPT_ffixed_x24))
+Features.push_back("+reserve-x24");
+
+  if (Args.hasArg(options::OPT_ffixed_x26))
+Features.push_back("+reserve-x26");
+
+  if (Args.hasArg(options::OPT_ffixed_x27))
+Features.push_back("+reserve-x27");
+
+  if (Args.hasArg(options::OPT_ffixed_x28))
+Features.push_back("+reserve-x28");
+
   if (Args.hasArg(options::OPT_fcall_saved_x8))
 Features.push_back("+call-saved-x8");
 

Modified: cfe/trunk/test/Driver/aarch64-fixed-x-register.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-fixed-x-register.c?rev=353957&r1=353956&r2=353957&view=diff
==
--- cfe/trunk/test/Driver/aarch64-fixed-x-register.c (original)
+++ cfe/trunk/test/Driver/aarch64-fixed-x-register.c Wed Feb 13 09:28:47 2019
@@ -26,6 +26,34 @@
 // RUN: FileCheck --check-prefix=CHECK-FIXED-X7 < %t %s
 // CHECK-FIXED-X7: "-target-feature" "+reserve-x7"
 
+// RUN: %clang -target aarch64-none-gnu -ffixed-x9 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X9 < %t %s
+// CHECK-FIXED-X9: "-target-feature" "+reserve-x9"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x10 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X10 < %t %s
+// CHECK-FIXED-X10: "-target-feature" "+reserve-x10"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x11 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X11 < %t %s
+// CHECK-FIXED-X11: "-target-feature" "+reserve-x11"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x12 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X12 < %t %s
+// CHECK-FIXED-X12: "-target-feature" "+reserve-x12"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x13 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X13 < %t %s
+// CHECK-FIXED-X13: "-target-feature" "

[PATCH] D58091: Customize warnings for missing built-in type

[Brian Cain via Phabricator via cfe-commits]

bcain added a comment.

> Are you sure it is unrelated?

Very nearly positive, I applied the patch to our downstream trunk and the 
assertion is encountered in a target-specific pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58091



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

[PATCH] D56305: [AArch64] Support reserving arbitrary general purpose registers

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353957: [AArch64] Support reserving arbitrary general 
purpose registers (authored by phosek, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56305?vs=186583&id=186687#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D56305

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Arch/AArch64.cpp
  test/Driver/aarch64-fixed-x-register.c

Index: test/Driver/aarch64-fixed-x-register.c
===
--- test/Driver/aarch64-fixed-x-register.c
+++ test/Driver/aarch64-fixed-x-register.c
@@ -26,6 +26,34 @@
 // RUN: FileCheck --check-prefix=CHECK-FIXED-X7 < %t %s
 // CHECK-FIXED-X7: "-target-feature" "+reserve-x7"
 
+// RUN: %clang -target aarch64-none-gnu -ffixed-x9 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X9 < %t %s
+// CHECK-FIXED-X9: "-target-feature" "+reserve-x9"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x10 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X10 < %t %s
+// CHECK-FIXED-X10: "-target-feature" "+reserve-x10"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x11 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X11 < %t %s
+// CHECK-FIXED-X11: "-target-feature" "+reserve-x11"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x12 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X12 < %t %s
+// CHECK-FIXED-X12: "-target-feature" "+reserve-x12"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x13 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X13 < %t %s
+// CHECK-FIXED-X13: "-target-feature" "+reserve-x13"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x14 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X14 < %t %s
+// CHECK-FIXED-X14: "-target-feature" "+reserve-x14"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x15 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X15 < %t %s
+// CHECK-FIXED-X15: "-target-feature" "+reserve-x15"
+
 // RUN: %clang -target aarch64-none-gnu -ffixed-x18 -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-FIXED-X18 < %t %s
 // CHECK-FIXED-X18: "-target-feature" "+reserve-x18"
@@ -34,6 +62,38 @@
 // RUN: FileCheck --check-prefix=CHECK-FIXED-X20 < %t %s
 // CHECK-FIXED-X20: "-target-feature" "+reserve-x20"
 
+// RUN: %clang -target aarch64-none-gnu -ffixed-x21 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X21 < %t %s
+// CHECK-FIXED-X21: "-target-feature" "+reserve-x21"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x22 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X22 < %t %s
+// CHECK-FIXED-X22: "-target-feature" "+reserve-x22"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x23 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X23 < %t %s
+// CHECK-FIXED-X23: "-target-feature" "+reserve-x23"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x24 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X24 < %t %s
+// CHECK-FIXED-X24: "-target-feature" "+reserve-x24"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x25 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X25 < %t %s
+// CHECK-FIXED-X25: "-target-feature" "+reserve-x25"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x26 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X26 < %t %s
+// CHECK-FIXED-X26: "-target-feature" "+reserve-x26"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x27 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X27 < %t %s
+// CHECK-FIXED-X27: "-target-feature" "+reserve-x27"
+
+// RUN: %clang -target aarch64-none-gnu -ffixed-x28 -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X28 < %t %s
+// CHECK-FIXED-X28: "-target-feature" "+reserve-x28"
+
 // Test multiple of reserve-x# options together.
 // RUN: %clang -target aarch64-none-gnu \
 // RUN: -ffixed-x1 \
@@ -55,8 +115,23 @@
 // RUN: -ffixed-x5 \
 // RUN: -ffixed-x6 \
 // RUN: -ffixed-x7 \
+// RUN: -ffixed-x9 \
+// RUN: -ffixed-x10 \
+// RUN: -ffixed-x11 \
+// RUN: -ffixed-x12 \
+// RUN: -ffixed-x13 \
+// RUN: -ffixed-x14 \
+// RUN: -ffixed-x15 \
 // RUN: -ffixed-x18 \
 // RUN: -ffixed-x20 \
+// RUN: -ffixed-x21 \
+// RUN: -ffixed-x22 \
+// RUN: -ffixed-x23 \
+// RUN: -ffixed-x24 \
+// RUN: -ffixed-x25 \
+// RUN: -ffixed-x26 \
+// RUN: -ffixed-x27 \
+// RUN: -ffixed-x28 \
 // RUN: -### %s 2> %t
 // RUN: FileCheck \
 // RUN: --check-prefix=CHECK-FIXED-X1 \
@@ -66,6 +141,21 @@
 // RUN: --check-prefix=CHECK-FIXED-X5 \
 // RUN: --check-prefix=CHECK-FIXED-X6 \
 // RUN: --check-prefix=CHECK-FIXED-X7 \
+// RUN: --check-prefix=CHECK-FIXED-X9 \
+// RUN: --check-prefix=CHECK-FIXED-X10 \
+// RUN: --check-prefix=CHECK-FIXED-X11 \
+// RUN: --check-prefix=CHECK-FIXED-X12 \
+// RUN: --check-prefix=CHECK-FIXED-X13 \
+// RUN: --check-prefix=CHECK-FIXED-X14 \
+// RUN: --check-prefix=CHECK-FIXED-X15 \

[PATCH] D58178: [clang-tidy] RawStringLiteralCheck: isRawStringLiteral doesn't check all erroneous cases

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Rather than sending a review, probably the reproducible case as to why it crash 
is more important, it might be better to write up your bug at 
https://bugs.llvm.org/

I tried to repoduce this and I can't see how I could make a string literal 
without a double quote, if it still failed with the current code it might 
suggest the matcher matched something else.. understanding what that is is 
probably more important, than just masking the failure.

there was a crash D19331: [Clang-tidy] Fix for crash in 
modernize-raw-string-literal check  which was 
fixed back in April, are you able to tell us the version/revision of clang-tidy 
where this occurred


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58178



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

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: include/clang/Parse/Parser.h:382
+
+  friend ParsedStmtContext operator|(ParsedStmtContext A, ParsedStmtContext B) 
{
+return ParsedStmtContext((unsigned)A | (unsigned)B);

We have `llvm/ADT/BitmaskEnum.h`, which defines `LLVM_MARK_AS_BITMASK_ENUM` for 
the bitwise ops on the enums. Maybe it is better to use it rather than manually 
define the required operations?



Comment at: lib/CodeGen/CGStmt.cpp:401
+break;
+  } else if (const LabelStmt *LS = dyn_cast(LastStmt)) {
+EmitLabel(LS->getDecl());

You don't need `else if` here, just ]ша] is enough since there is `break;` in 
the previous substatement.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57984



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

[PATCH] D58178: [clang-tidy] RawStringLiteralCheck: isRawStringLiteral doesn't check all erroneous cases

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D58178#1396435 , @gmit wrote:

> ? I'm sorry, but I disagree.
>
> Assert draws attention in the debug build only.
>
> In the release build asserts are not evaluated at all and the condition needs 
> to be checked in the code that executes (if it makes sense and in this case 
> it does as it causes reading out of string bounds).


What i'm saying is, do you agree that the `assert(QuotePos != 
StringRef::npos);` happens before `return (QuotePos != StringRef::npos) && 
` ?
If you build with asserts enabled, that your original code which caused it to 
"read from memory randomly"
will trigger that assertion. Even with this fix. So the fix is not correct.

> I don't have a test as I don't have an input file that causes this behaviour.

It's quite easy to trim down the source that crashes down to something usable 
with https://embed.cs.utah.edu/creduce/
The end result won't resemble anything the original code was, no proprietary 
secrets will be leaked.
Without test case, there is nothing do to here.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58178



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

[PATCH] D58060: Fix diagnostic for addr spaces in static_cast

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D58060#1396416 , @ebevhan wrote:

> So static_cast permits conversions from AS1 to AS2 where that conversion is 
> implicitly allowed, and the new addrspace_cast would permit conversions from 
> AS1 to AS2 where it is explicitly allowed. That seems like it fits in rather 
> well with the idea in D57464  regarding 
> support for specifying permitted AS conversions in target.
>
> How about nested pointers, such as `__X int * *` -> `__Y int * *` or `__X int 
> * __Y *` -> `int * __Y *`? static_cast has some ruleset for how to deal with 
> qualifiers in nested pointers, I think, but I'm not sure how the rules for 
> ASes should be here.


We had discussion related to this with John earlier. And I documented it in 
this bug: https://bugs.llvm.org/show_bug.cgi?id=39674

> There's also C-style casts. Will addrspace_cast be added to the C-style cast 
> rules somewhere?
> 
> (I should probably have commented on the original RFC)

Yes, exactly. I am working on a patch now that separates the address space 
casting out into a function to be used in C-style cast only as a first step 
(because I would like to disallow accidental unsafe address space conversions 
in `reinterpret_cast`). Then we would just need to add parsing of a new cast 
operator and mapping into the new function.


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

https://reviews.llvm.org/D58060



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

[PATCH] D58189: [clang][Index] Fix usage of IndexImplicitInstantiation

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Only have a few NITs, will dig deeper into the change tomorrow.
Added @arphaman too as an owner of the index library. Alex, feel free to 
reassign if you're the wrong person to take a look at this




Comment at: unittests/Index/IndexTests.cpp:31
+struct Position {
+  size_t Line;
+  size_t Column;

NIT: initialize with 0 to avoid UB.



Comment at: unittests/Index/IndexTests.cpp:40
+Position P;
+P.Line = static_cast(SM.getLineNumber(FID, Offset)) - 1;
+P.Column = SM.getColumnNumber(FID, Offset) - 1;

Why do we need to `static_cast` to int? Can we leave out the cast?



Comment at: unittests/Index/IndexTests.cpp:45
+
+  bool operator==(const Position &Other) const {
+return std::tie(Line, Column) == std::tie(Other.Line, Other.Column);

NIT: maybe make it a free-standing function, accepting two parameters:
```
struct Position {
  friend bool operator==(const Pos& L, const Pos& R) {
// ...
  }
};
```
Doesn't really matter much here, though, just a general best practice.



Comment at: unittests/Index/IndexTests.cpp:97
   std::vector Symbols;
+  const ASTContext *AST;
 };

NIT: initialize with null to make UB less likely


Repository:
  rC Clang

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

https://reviews.llvm.org/D58189



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

r353960 - [Driver] Pass +reserve-x25 to backend if -ffixed-x25 is specified

[Petr Hosek via cfe-commits]

Author: phosek
Date: Wed Feb 13 10:01:23 2019
New Revision: 353960

URL: http://llvm.org/viewvc/llvm-project?rev=353960&view=rev
Log:
[Driver] Pass +reserve-x25 to backend if -ffixed-x25 is specified

This was accidentally omitted in r353957 breaking the Clang test.

Modified:
cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp?rev=353960&r1=353959&r2=353960&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp Wed Feb 13 10:01:23 2019
@@ -373,6 +373,9 @@ fp16_fml_fallthrough:
   if (Args.hasArg(options::OPT_ffixed_x24))
 Features.push_back("+reserve-x24");
 
+  if (Args.hasArg(options::OPT_ffixed_x25))
+Features.push_back("+reserve-x25");
+
   if (Args.hasArg(options::OPT_ffixed_x26))
 Features.push_back("+reserve-x26");
 


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

[PATCH] D58134: [Analysis] -Wunreachable-code shouldn't fire on the increment of a foreach loop

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D58134



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

r353965 - [Headers][mips] Add `__attribute__((__mode__(__unwind_word__)))` to the _Unwind_Word / _Unwind_SWord definitions

[Simon Atanasyan via cfe-commits]

Author: atanasyan
Date: Wed Feb 13 10:27:09 2019
New Revision: 353965

URL: http://llvm.org/viewvc/llvm-project?rev=353965&view=rev
Log:
[Headers][mips] Add `__attribute__((__mode__(__unwind_word__)))` to the 
_Unwind_Word / _Unwind_SWord definitions

The rationale of this change is to fix _Unwind_Word / _Unwind_SWord
definitions for MIPS N32 ABI. This ABI uses 32-bit pointers,
but _Unwind_Word and _Unwind_SWord types are eight bytes long.

 # The __attribute__((__mode__(__unwind_word__))) is added to the type
   definitions. It makes them equal to the corresponding definitions used
   by GCC and allows to override types using `getUnwindWordWidth` function.
 # The `getUnwindWordWidth` virtual function override in the `MipsTargetInfo`
   class and provides correct type size values.

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

Modified:
cfe/trunk/lib/Basic/Targets/Mips.cpp
cfe/trunk/lib/Basic/Targets/Mips.h
cfe/trunk/lib/Headers/unwind.h
cfe/trunk/test/Sema/attr-mode.c

Modified: cfe/trunk/lib/Basic/Targets/Mips.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/Mips.cpp?rev=353965&r1=353964&r2=353965&view=diff
==
--- cfe/trunk/lib/Basic/Targets/Mips.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/Mips.cpp Wed Feb 13 10:27:09 2019
@@ -215,6 +215,14 @@ ArrayRef MipsTargetInfo::
  Builtin::FirstTSBuiltin);
 }
 
+unsigned MipsTargetInfo::getUnwindWordWidth() const {
+  return llvm::StringSwitch(ABI)
+  .Case("o32", 32)
+  .Case("n32", 64)
+  .Case("n64", 64)
+  .Default(getPointerWidth(0));
+}
+
 bool MipsTargetInfo::validateTarget(DiagnosticsEngine &Diags) const {
   // microMIPS64R6 backend was removed.
   if (getTriple().isMIPS64() && IsMicromips && (ABI == "n32" || ABI == "n64")) 
{

Modified: cfe/trunk/lib/Basic/Targets/Mips.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/Mips.h?rev=353965&r1=353964&r2=353965&view=diff
==
--- cfe/trunk/lib/Basic/Targets/Mips.h (original)
+++ cfe/trunk/lib/Basic/Targets/Mips.h Wed Feb 13 10:27:09 2019
@@ -401,6 +401,8 @@ public:
 return (ABI == "n32" || ABI == "n64") || getTargetOpts().ForceEnableInt128;
   }
 
+  unsigned getUnwindWordWidth() const override;
+
   bool validateTarget(DiagnosticsEngine &Diags) const override;
 };
 } // namespace targets

Modified: cfe/trunk/lib/Headers/unwind.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/unwind.h?rev=353965&r1=353964&r2=353965&view=diff
==
--- cfe/trunk/lib/Headers/unwind.h (original)
+++ cfe/trunk/lib/Headers/unwind.h Wed Feb 13 10:27:09 2019
@@ -66,8 +66,8 @@ extern "C" {
 #pragma GCC visibility push(default)
 #endif
 
-typedef uintptr_t _Unwind_Word;
-typedef intptr_t _Unwind_Sword;
+typedef uintptr_t _Unwind_Word __attribute__((__mode__(__unwind_word__)));
+typedef intptr_t _Unwind_Sword __attribute__((__mode__(__unwind_word__)));
 typedef uintptr_t _Unwind_Ptr;
 typedef uintptr_t _Unwind_Internal_Ptr;
 typedef uint64_t _Unwind_Exception_Class;

Modified: cfe/trunk/test/Sema/attr-mode.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-mode.c?rev=353965&r1=353964&r2=353965&view=diff
==
--- cfe/trunk/test/Sema/attr-mode.c (original)
+++ cfe/trunk/test/Sema/attr-mode.c Wed Feb 13 10:27:09 2019
@@ -6,6 +6,12 @@
 // RUN:   -verify %s
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnux32 -DTEST_64BIT_X86 
-fsyntax-only \
 // RUN:   -verify %s
+// RUN: %clang_cc1 -triple mips-linux-gnu -DTEST_MIPS_32 -fsyntax-only \
+// RUN:   -verify %s
+// RUN: %clang_cc1 -triple mips64-linux-gnuabin32 -DTEST_MIPS_N32 
-fsyntax-only \
+// RUN:   -verify %s
+// RUN: %clang_cc1 -triple mips64-linux-gnu -DTEST_MIPS_64 -fsyntax-only \
+// RUN:   -verify %s
 
 typedef int i16_1 __attribute((mode(HI)));
 int i16_1_test[sizeof(i16_1) == 2 ? 1 : -1];
@@ -33,7 +39,7 @@ typedef _Complex double c32 __attribute(
 int c32_test[sizeof(c32) == 8 ? 1 : -1];
 typedef _Complex float c64 __attribute((mode(DC)));
 
-#ifndef TEST_64BIT_PPC64 // Note, 'XC' mode is illegal for PPC64 machines.
+#if !defined(__ppc__) && !defined(__mips__) // Note, 'XC' mode is illegal for 
PPC64 and MIPS machines.
 typedef _Complex float c80 __attribute((mode(XC)));
 #endif
 
@@ -84,6 +90,15 @@ void f_ft128_arg(long double *x);
 void f_ft128_complex_arg(_Complex long double *x);
 void test_TFtype(f128ibm *a) { f_ft128_arg (a); }
 void test_TCtype(c128ibm *a) { f_ft128_complex_arg (a); }
+#elif TEST_MIPS_32
+typedef unsigned int gcc_unwind_word __attribute__((mode(unwind_word)));
+int foo[sizeof(gcc_unwind_word) == 4 ? 1 : -1];
+#elif TEST_MIPS_N32
+typedef unsigned int gcc_unwind_word __attribute__((mode(unwind_word

[PATCH] D58178: [clang-tidy] RawStringLiteralCheck: isRawStringLiteral doesn't check all erroneous cases

[Goran Mitrovic via Phabricator via cfe-commits]

gmit added a comment.

In D58178#1396523 , @MyDeveloperDay 
wrote:

> I tried to repoduce this and I can't see how I could make a string literal 
> without a double quote, if it still failed with the current code it might 
> suggest the matcher matched something else.. understanding what that is is 
> probably more important, than just masking the failure.


I agree to that. There is not obvious reason why this would happen.

> there was a crash D19331: [Clang-tidy] Fix for crash in 
> modernize-raw-string-literal check  which 
> was fixed back in April, are you able to tell us the version/revision of 
> clang-tidy where this occurred

The crash happened on Clang 5 and I can confirm fix 19331 was present in the 
source files that built it.

> It's quite easy to trim down the source that crashes down to something usable 
> with https://embed.cs.utah.edu/creduce/
>  The end result won't resemble anything the original code was, no proprietary 
> secrets will be leaked.
>  Without test case, there is nothing do to here.

No, it's not that - I simply don't have it. I don't have .dmp either. I only 
have our fix that was done in our local Clang repository copy two years ago and 
a call stack.

Anyhow, I agree to dismiss this case. Sorry for wasting your time! Thanks!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58178



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

[PATCH] D58164: Block+lambda: allow reference capture

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

The code is crashing here because the loop in `computeBlockInfo` is trying to 
capture a variable that is captured by reference by the enclosing lambda as if 
it were captured by value. This is the type of VT:

  LValueReferenceType 0x1138008c0 'struct derp &'
  `-RecordType 0x113052580 'struct derp'
`-CXXRecord 0x1130524e8 'derp'

So in this case, the type doesn't require non-trivial copy construction.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58164



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

[PATCH] D58163: [CUDA][HIP] Use device side kernel and variable names when registering them

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: lib/CodeGen/CGCUDANV.cpp:128
+ unsigned Flags) override {
+DeviceVars.push_back(VarInfo{&Var, getDeviceSideName(VD), Flags});
   }

Nit: `VarInfo` is not needed. Compiler should be able to infer it.



Comment at: lib/CodeGen/CGCUDANV.cpp:412
+  for (auto &&I : EmittedKernels) {
+llvm::Constant *KernelName = makeConstantString(I.DeviceSideName);
 llvm::Constant *NullPtr = llvm::ConstantPointerNull::get(VoidPtrTy);

Can we just call `getDeviceSideName()` here ? Mangling the name early and 
carrying it around does not seem to buy us anything.


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

https://reviews.llvm.org/D58163



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

[PATCH] D58165: [Headers][mips] Add `__attribute__((__mode__(__unwind_word__)))` to the _Unwind_Word / _Unwind_SWord definitions

[Simon Atanasyan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353965: [Headers][mips] Add 
`__attribute__((__mode__(__unwind_word__)))` to the… (authored by atanasyan, 
committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58165?vs=186585&id=186694#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58165

Files:
  lib/Basic/Targets/Mips.cpp
  lib/Basic/Targets/Mips.h
  lib/Headers/unwind.h
  test/Sema/attr-mode.c


Index: lib/Basic/Targets/Mips.cpp
===
--- lib/Basic/Targets/Mips.cpp
+++ lib/Basic/Targets/Mips.cpp
@@ -215,6 +215,14 @@
  Builtin::FirstTSBuiltin);
 }
 
+unsigned MipsTargetInfo::getUnwindWordWidth() const {
+  return llvm::StringSwitch(ABI)
+  .Case("o32", 32)
+  .Case("n32", 64)
+  .Case("n64", 64)
+  .Default(getPointerWidth(0));
+}
+
 bool MipsTargetInfo::validateTarget(DiagnosticsEngine &Diags) const {
   // microMIPS64R6 backend was removed.
   if (getTriple().isMIPS64() && IsMicromips && (ABI == "n32" || ABI == "n64")) 
{
Index: lib/Basic/Targets/Mips.h
===
--- lib/Basic/Targets/Mips.h
+++ lib/Basic/Targets/Mips.h
@@ -401,6 +401,8 @@
 return (ABI == "n32" || ABI == "n64") || getTargetOpts().ForceEnableInt128;
   }
 
+  unsigned getUnwindWordWidth() const override;
+
   bool validateTarget(DiagnosticsEngine &Diags) const override;
 };
 } // namespace targets
Index: lib/Headers/unwind.h
===
--- lib/Headers/unwind.h
+++ lib/Headers/unwind.h
@@ -66,8 +66,8 @@
 #pragma GCC visibility push(default)
 #endif
 
-typedef uintptr_t _Unwind_Word;
-typedef intptr_t _Unwind_Sword;
+typedef uintptr_t _Unwind_Word __attribute__((__mode__(__unwind_word__)));
+typedef intptr_t _Unwind_Sword __attribute__((__mode__(__unwind_word__)));
 typedef uintptr_t _Unwind_Ptr;
 typedef uintptr_t _Unwind_Internal_Ptr;
 typedef uint64_t _Unwind_Exception_Class;
Index: test/Sema/attr-mode.c
===
--- test/Sema/attr-mode.c
+++ test/Sema/attr-mode.c
@@ -6,6 +6,12 @@
 // RUN:   -verify %s
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnux32 -DTEST_64BIT_X86 
-fsyntax-only \
 // RUN:   -verify %s
+// RUN: %clang_cc1 -triple mips-linux-gnu -DTEST_MIPS_32 -fsyntax-only \
+// RUN:   -verify %s
+// RUN: %clang_cc1 -triple mips64-linux-gnuabin32 -DTEST_MIPS_N32 
-fsyntax-only \
+// RUN:   -verify %s
+// RUN: %clang_cc1 -triple mips64-linux-gnu -DTEST_MIPS_64 -fsyntax-only \
+// RUN:   -verify %s
 
 typedef int i16_1 __attribute((mode(HI)));
 int i16_1_test[sizeof(i16_1) == 2 ? 1 : -1];
@@ -33,7 +39,7 @@
 int c32_test[sizeof(c32) == 8 ? 1 : -1];
 typedef _Complex float c64 __attribute((mode(DC)));
 
-#ifndef TEST_64BIT_PPC64 // Note, 'XC' mode is illegal for PPC64 machines.
+#if !defined(__ppc__) && !defined(__mips__) // Note, 'XC' mode is illegal for 
PPC64 and MIPS machines.
 typedef _Complex float c80 __attribute((mode(XC)));
 #endif
 
@@ -84,6 +90,15 @@
 void f_ft128_complex_arg(_Complex long double *x);
 void test_TFtype(f128ibm *a) { f_ft128_arg (a); }
 void test_TCtype(c128ibm *a) { f_ft128_complex_arg (a); }
+#elif TEST_MIPS_32
+typedef unsigned int gcc_unwind_word __attribute__((mode(unwind_word)));
+int foo[sizeof(gcc_unwind_word) == 4 ? 1 : -1];
+#elif TEST_MIPS_N32
+typedef unsigned int gcc_unwind_word __attribute__((mode(unwind_word)));
+int foo[sizeof(gcc_unwind_word) == 8 ? 1 : -1];
+#elif TEST_MIPS_64
+typedef unsigned int gcc_unwind_word __attribute__((mode(unwind_word)));
+int foo[sizeof(gcc_unwind_word) == 8 ? 1 : -1];
 #else
 #error Unknown test architecture.
 #endif


Index: lib/Basic/Targets/Mips.cpp
===
--- lib/Basic/Targets/Mips.cpp
+++ lib/Basic/Targets/Mips.cpp
@@ -215,6 +215,14 @@
  Builtin::FirstTSBuiltin);
 }
 
+unsigned MipsTargetInfo::getUnwindWordWidth() const {
+  return llvm::StringSwitch(ABI)
+  .Case("o32", 32)
+  .Case("n32", 64)
+  .Case("n64", 64)
+  .Default(getPointerWidth(0));
+}
+
 bool MipsTargetInfo::validateTarget(DiagnosticsEngine &Diags) const {
   // microMIPS64R6 backend was removed.
   if (getTriple().isMIPS64() && IsMicromips && (ABI == "n32" || ABI == "n64")) {
Index: lib/Basic/Targets/Mips.h
===
--- lib/Basic/Targets/Mips.h
+++ lib/Basic/Targets/Mips.h
@@ -401,6 +401,8 @@
 return (ABI == "n32" || ABI == "n64") || getTargetOpts().ForceEnableInt128;
   }
 
+  unsigned getUnwindWordWidth() const override;
+
   bool validateTarget(DiagnosticsEngine &Diags) const override;
 };
 } // namespace targets
Index: lib/Headers/unwind.h
=

[PATCH] D58164: Block+lambda: allow reference capture

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I think the root of the problem is that `BlockDecl` knows the captured variable 
but doesn't know the type of the capture. The type of the variable and the type 
of the capture can be different if the block is nested inside a lambda or 
another block, which is why `CI.hasCopyExpr()` can return a non-null value when 
`VT` is a lvalue reference:

  `-FunctionDecl 0x11306dc28  line:11:6 test 'void ()'
  |-DeclStmt 0x11306ddb0 
  | `-VarDecl 0x11306dd20  col:8 used c 'derp' callinit
  ...
  | |-CXXRecordDecl 0x11306dec0  col:12 implicit class 
definition
  ...
  | | |-FieldDecl 0x11309cc58  col:7 implicit 'derp &'
  ...
  | | |   |   `-BlockDecl 0x11309cb28  
line:15:18
  | | |   | |-capture nested Var 0x11306dd20 'c' 'derp'


Repository:
  rC Clang

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

https://reviews.llvm.org/D58164



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

[PATCH] D58189: [clang][Index] Fix usage of IndexImplicitInstantiation

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: unittests/Index/IndexTests.cpp:30
 
+struct Position {
+  size_t Line;

NIT: put all of the decls of a file into an anonymous namespace


Repository:
  rC Clang

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

https://reviews.llvm.org/D58189



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

[PATCH] D58195: [HWASAN] Updated HWASAN design document to better portray the chance of missing a bug.

[Mitch Phillips via Phabricator via cfe-commits]

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

Provided rule of thumb percentage chances of miss for 4 and 8 bit tag sizes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58195

Files:
  clang/docs/HardwareAssistedAddressSanitizerDesign.rst


Index: clang/docs/HardwareAssistedAddressSanitizerDesign.rst
===
--- clang/docs/HardwareAssistedAddressSanitizerDesign.rst
+++ clang/docs/HardwareAssistedAddressSanitizerDesign.rst
@@ -131,7 +131,8 @@
 https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt).
   * **Does not require redzones to detect buffer overflows**,
 but the buffer overflow detection is probabilistic, with roughly
-`(2**TS-1)/(2**TS)` probability of catching a bug.
+`1/(2**TS)` chance of missing a bug (6.25% or 0.39% with 4 and 8-bit TS
+respectively).
   * **Does not require quarantine to detect heap-use-after-free,
 or stack-use-after-return**.
 The detection is similarly probabilistic.


Index: clang/docs/HardwareAssistedAddressSanitizerDesign.rst
===
--- clang/docs/HardwareAssistedAddressSanitizerDesign.rst
+++ clang/docs/HardwareAssistedAddressSanitizerDesign.rst
@@ -131,7 +131,8 @@
 https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt).
   * **Does not require redzones to detect buffer overflows**,
 but the buffer overflow detection is probabilistic, with roughly
-`(2**TS-1)/(2**TS)` probability of catching a bug.
+`1/(2**TS)` chance of missing a bug (6.25% or 0.39% with 4 and 8-bit TS
+respectively).
   * **Does not require quarantine to detect heap-use-after-free,
 or stack-use-after-return**.
 The detection is similarly probabilistic.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

Any comments?


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

https://reviews.llvm.org/D57662



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

[PATCH] D53928: Enable builtins necessary for SLEEF [AArch64] vectorized trigonometry libm functions

[Stefan Teleman via Phabricator via cfe-commits]

steleman added a comment.

Hi Renato,

Thank you very much for the comments.

I will create a new changeset incorporating your comments, and then re-submit.

I will also add a Diagnostic for SVML and X86/X86_64.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53928



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

[PATCH] D58060: Fix diagnostic for addr spaces in static_cast

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 186699.
Anastasia added a comment.
Herald added a subscriber: javed.absar.

- Changed the diagnostic for binding reference and combined with existing 
similar one. That affected more tests however.
- Changed comment explaining address space behavior in the reference 
initialization.
- Reformatted.


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

https://reviews.llvm.org/D58060

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaInit.cpp
  test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-0x.cpp
  test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-var.cpp
  test/CXX/expr/expr.post/expr.static.cast/p3-p4-0x.cpp
  test/CXX/expr/expr.prim/expr.prim.lambda/p16.cpp
  test/Misc/diag-template-diffing.cpp
  test/SemaCXX/builtins-arm.cpp
  test/SemaCXX/err_reference_bind_drops_quals.cpp
  test/SemaCXX/references.cpp
  test/SemaOpenCLCXX/address-space-castoperators.cpp

Index: test/SemaOpenCLCXX/address-space-castoperators.cpp
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-castoperators.cpp
@@ -0,0 +1,17 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify
+
+void test_ref(int &gen, __global const int &glob) {
+  static_cast<__global int &>(gen); // expected-error{{binding reference of type '__global int' to value of type '__generic int' changes address space}}
+  static_cast<__global const int &>(gen);   //expected-error{{binding reference of type 'const __global int' to value of type '__generic int' changes address space}}
+  static_cast<__global int &>(glob);//expected-error{{binding reference of type '__global int' to value of type 'const __global int' drops 'const' qualifier}}
+  static_cast<__local int &>(glob); //expected-error{{binding reference of type '__local int' to value of type 'const __global int' changes address space}}
+  static_cast<__generic const int &>(glob); //expected-warning{{expression result unused}}
+}
+
+void test_ptr(int *gen, __global const int *glob) {
+  static_cast<__global int *>(gen); // expected-error{{static_cast from '__generic int *' to '__global int *' is not allowed}}
+  static_cast<__global const int *>(gen);   //expected-error{{static_cast from '__generic int *' to 'const __global int *' is not allowed}}
+  static_cast<__global int *>(glob);//expected-error{{static_cast from 'const __global int *' to '__global int *' is not allowed}}
+  static_cast<__local int *>(glob); //expected-error{{static_cast from 'const __global int *' to '__local int *' is not allowed}}
+  static_cast<__generic const int *>(glob); //expected-warning{{expression result unused}}
+}
Index: test/SemaCXX/references.cpp
===
--- test/SemaCXX/references.cpp
+++ test/SemaCXX/references.cpp
@@ -55,13 +55,13 @@
 void test5() {
   //  const double& rcd2 = 2; // rcd2 refers to temporary with value 2.0
   const volatile int cvi = 1;
-  const int& r = cvi; // expected-error{{binding value of type 'const volatile int' to reference to type 'const int' drops 'volatile' qualifier}}
+  const int& r = cvi; // expected-error{{binding reference of type 'const int' to value of type 'const volatile int' drops 'volatile' qualifier}}
 
 #if __cplusplus >= 201103L
-  const int& r2{cvi}; // expected-error{{binding value of type 'const volatile int' to reference to type 'const int' drops 'volatile' qualifier}}
+  const int& r2{cvi}; // expected-error{{binding reference of type 'const int' to value of type 'const volatile int' drops 'volatile' qualifier}}
 
   const int a = 2;
-  int& r3{a}; // expected-error{{binding value of type 'const int' to reference to type 'int' drops 'const'}}
+  int& r3{a}; // expected-error{{binding reference of type 'int' to value of type 'const int' drops 'const' qualifier}}
 
   const int&& r4{a}; // expected-error{{rvalue reference to type 'const int' cannot bind to lvalue of type 'const int'}}
 
Index: test/SemaCXX/err_reference_bind_drops_quals.cpp
===
--- test/SemaCXX/err_reference_bind_drops_quals.cpp
+++ test/SemaCXX/err_reference_bind_drops_quals.cpp
@@ -6,31 +6,31 @@
volatile ptr vp, const volatile ptr cvp, restrict volatile ptr rvp,
const restrict volatile ptr crvp) {
   ptr& p1 = p;
-  ptr& p2 = cp; // expected-error {{drops 'const' qualifier}}
-  ptr& p3 = rp; // expected-error {{drops 'restrict' qualifier}}
-  ptr& p4 = crp; // expected-error {{drops 'const' and 'restrict' qualifiers}}
-  ptr& p5 = vp; // expected-error {{drops 'volatile' qualifier}}
-  ptr& p6 = cvp; // expected-error {{drops 'const' and 'volatile' qualifiers}}
-  ptr& p7 = rvp; // expected-error {{drops 'restrict' and 'volatile' qualifiers}}
-  ptr& p8 = crvp; // expected-error {{drops 'const', 'restrict', and 'volatile' qualifiers}}
+  ptr& p2 = cp;   // expecte

r353969 - Re-enable the test disabled in r353836 and hopefully make it pass in gcc builds

[Nico Weber via cfe-commits]

Author: nico
Date: Wed Feb 13 11:04:26 2019
New Revision: 353969

URL: http://llvm.org/viewvc/llvm-project?rev=353969&view=rev
Log:
Re-enable the test disabled in r353836 and hopefully make it pass in gcc builds

Argument evaluation order is different between gcc and clang, so pull out
the Builder calls to make the generated IR independent of the host compiler's
argument evaluation order.  Thanks to rnk for reminding me of this clang/gcc
difference.

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/ms-x86-intrinsics.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=353969&r1=353968&r2=353969&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Feb 13 11:04:26 2019
@@ -11854,9 +11854,10 @@ Value *CodeGenFunction::EmitX86BuiltinEx
 // Ops[2] = Builder.CreateZExt(Ops[2], Int64Ty);
 // return Builder.CreateCall(F, Ops);
 llvm::Type *Int128Ty = Builder.getInt128Ty();
-Value *Val = Builder.CreateOr(
-Builder.CreateShl(Builder.CreateZExt(Ops[1], Int128Ty), 64),
-Builder.CreateZExt(Ops[0], Int128Ty));
+Value *HighPart128 =
+Builder.CreateShl(Builder.CreateZExt(Ops[1], Int128Ty), 64);
+Value *LowPart128 = Builder.CreateZExt(Ops[0], Int128Ty);
+Value *Val = Builder.CreateOr(HighPart128, LowPart128);
 Value *Amt = Builder.CreateAnd(Builder.CreateZExt(Ops[2], Int128Ty),
llvm::ConstantInt::get(Int128Ty, 0x3f));
 Value *Res;

Modified: cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-x86-intrinsics.c?rev=353969&r1=353968&r2=353969&view=diff
==
--- cfe/trunk/test/CodeGen/ms-x86-intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/ms-x86-intrinsics.c Wed Feb 13 11:04:26 2019
@@ -143,33 +143,29 @@ unsigned __int64 test__shiftleft128(unsi
 unsigned char d) {
   return __shiftleft128(l, h, d);
 }
-// FIXME: Add ':' after all the CHECK-X64 lines here once it's understood
-// why the order of the output is different when using clang or gcc as host cc.
 // CHECK-X64-LABEL: define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, 
i8 %d)
 // CHECK-X64:  = zext i64 %{{.*}} to i128
-// CHECK-X64  = shl nuw i128 %{{.*}}, 64
-// CHECK-X64  = zext i64 %{{.*}} to i128
-// CHECK-X64  = or i128 %
-// CHECK-X64  = and i8 %{{.*}}, 63
-// CHECK-X64  = shl i128 %
-// CHECK-X64  = lshr i128 %
-// CHECK-X64  = trunc i128 %
+// CHECK-X64: = shl nuw i128 %{{.*}}, 64
+// CHECK-X64: = zext i64 %{{.*}} to i128
+// CHECK-X64: = or i128 %
+// CHECK-X64: = and i8 %{{.*}}, 63
+// CHECK-X64: = shl i128 %
+// CHECK-X64: = lshr i128 %
+// CHECK-X64: = trunc i128 %
 // CHECK-X64:  ret i64 %
 
 unsigned __int64 test__shiftright128(unsigned __int64 l, unsigned __int64 h,
  unsigned char d) {
   return __shiftright128(l, h, d);
 }
-// FIXME: Add ':' after all the CHECK-X64 lines here once it's understood
-// why the order of the output is different when using clang or gcc as host cc.
 // CHECK-X64-LABEL: define dso_local i64 @test__shiftright128(i64 %l, i64 %h, 
i8 %d)
 // CHECK-X64:  = zext i64 %{{.*}} to i128
-// CHECK-X64  = shl nuw i128 %{{.*}}, 64
-// CHECK-X64  = zext i64 %{{.*}} to i128
-// CHECK-X64  = or i128 %
-// CHECK-X64  = and i8 %{{.*}}, 63
-// CHECK-X64  = lshr i128 %
-// CHECK-X64  = trunc i128 %
+// CHECK-X64: = shl nuw i128 %{{.*}}, 64
+// CHECK-X64: = zext i64 %{{.*}} to i128
+// CHECK-X64: = or i128 %
+// CHECK-X64: = and i8 %{{.*}}, 63
+// CHECK-X64: = lshr i128 %
+// CHECK-X64: = trunc i128 %
 // CHECK-X64:  ret i64 %
 
 #endif // defined(__x86_64__)


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

Re: r353729 - Attempt to pacify bots more after r353718 and r353725

[Nico Weber via cfe-commits]

I re-enabled the test in r353969, hopefully it'll pass when using gcc as
host too this time.

On Tue, Feb 12, 2019 at 9:10 AM Mikael Holmén 
wrote:

>
>
> On 2/12/19 1:41 PM, Nico Weber wrote:
> > Thanks for reporting that this depends on the host compiler.
> >
> > I disabled the test again in r353836 and will look into why the output
> > is different depending on if host cc is gcc or clang.
> >
>
> Good. Thanks!
>
> When I've seen things like this before it's often something like
> iterating over a set or similar where the iteration order isn't
> deterministic.
>
> /Mikael
>
> > On Tue, Feb 12, 2019 at 2:40 AM Mikael Holmén
> > mailto:mikael.hol...@ericsson.com>> wrote:
> >
> > Same thing for me with our downstream build bots.
> >
> > When we compile clang with gcc 7.4 and then run the testcase I get
> this
> > output
> >
> > define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, i8 %d)
> > local_unnamed_addr #0 {
> > entry:
> > %0 = zext i64 %l to i128
> > %1 = zext i64 %h to i128
> > %2 = shl nuw i128 %1, 64
> > %3 = or i128 %2, %0
> > %4 = and i8 %d, 63
> > %5 = zext i8 %4 to i128
> > %6 = shl i128 %3, %5
> > %7 = lshr i128 %6, 64
> > %8 = trunc i128 %7 to i64
> > ret i64 %8
> > }
> >
> > and when I compile clang with clang 3.6 and run the test I get this:
> >
> > define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, i8 %d)
> > local_unnamed_addr #0 {
> > entry:
> > %0 = zext i64 %h to i128
> > %1 = shl nuw i128 %0, 64
> > %2 = zext i64 %l to i128
> > %3 = or i128 %1, %2
> > %4 = and i8 %d, 63
> > %5 = zext i8 %4 to i128
> > %6 = shl i128 %3, %5
> > %7 = lshr i128 %6, 64
> > %8 = trunc i128 %7 to i64
> > ret i64 %8
> > }
> >
> > /Mikael
> >
> > On 2/12/19 2:03 AM, Nico Weber via cfe-commits wrote:
> >  > Thank you for the .ll files!
> >  >
> >  > the -4.ll file you sent me contains:
> >  >
> >  > define dso_local i64 @"?test__shiftleft128@@YA_K_K0E@Z"(i64 %l,
> > i64 %h,
> >  > i8 %d) local_unnamed_addr #0 {
> >  > entry:
> >  >%0 = zext i64 %h to i128
> >  >%1 = shl nuw i128 %0, 64
> >  >%2 = zext i64 %l to i128
> >  >%3 = or i128 %1, %2
> >  >%4 = and i8 %d, 63
> >  >%5 = zext i8 %4 to i128
> >  >%6 = shl i128 %3, %5
> >  >%7 = lshr i128 %6, 64
> >  >%8 = trunc i128 %7 to i64
> >  >ret i64 %8
> >  > }
> >  >
> >  > On my local system, I get
> >  >
> >  > ; Function Attrs: minsize norecurse nounwind optsize readnone
> >  > define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, i8 %d)
> >  > local_unnamed_addr #0 {
> >  > entry:
> >  >%0 = zext i64 %l to i128
> >  >%1 = zext i64 %h to i128
> >  >%2 = shl nuw i128 %1, 64
> >  >%3 = or i128 %2, %0
> >  >%4 = and i8 %d, 63
> >  >%5 = zext i8 %4 to i128
> >  >%6 = shl i128 %3, %5
> >  >%7 = lshr i128 %6, 64
> >  >%8 = trunc i128 %7 to i64
> >  >ret i64 %8
> >  > }
> >  >
> >  > That's identical except for the order of the instructions (which
> > in turn
> >  > changes some % numbers).
> >  >
> >  > That's surprising to me; I thought LLVM IR is deterministic (and
> > if it
> >  > wasn't, many other tests wouldn't work either).
> >  >
> >  > On Mon, Feb 11, 2019 at 4:20 PM Galina Kistanova
> > mailto:gkistan...@gmail.com>
> >  > >>
> wrote:
> >  >
> >  > Hello Nico,
> >  >
> >  > This builders fail on your test as well -
> >  >
> >
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/15736
> ,
> >  >
> http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/4242.
> >  >
> >  > Please find attached the 2 temp files you can use to reliably
> run
> >  > against your FileCheck patterns.
> >  > Hope this would help debugging.
> >  >
> >  > Please also notice the warnings each of the RUN command
> produces.
> >  > The warnings should be quite easy to reproduce and address.
> >  >
> >  > In the mean time, could you revert the change unless you
> > expect the
> >  > fix coming really soon, please?
> >  > It is not a good idea to keep the bots red for long.
> >  >
> >  > Here is the log:
> >  >
> >
>  --
> >  >
> >  >
> >
>  
> C:\>c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.exe
> >  > -cc1 -internal-isystem
> >  >
> >
>  
> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\9.0.0\include
> >   

[PATCH] D58149: [clang] Make sure C99/C11 features in are provided in C++11

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I'll ship this. @eli.friedman I think this is your playground -- if you want 
any changes to happen post-review, please LMK and I will gladly cooperate.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58149



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

r353970 - [clang] Make sure C99/C11 features in are provided in C++11

[Louis Dionne via cfe-commits]

Author: ldionne
Date: Wed Feb 13 11:08:01 2019
New Revision: 353970

URL: http://llvm.org/viewvc/llvm-project?rev=353970&view=rev
Log:
[clang] Make sure C99/C11 features in  are provided in C++11

Summary:
Previously, those #defines were only provided in C or when GNU extensions were
enabled. We need those #defines in C++11 and above, too.

Reviewers: jfb, eli.friedman

Subscribers: jkorous, dexonsmith, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Headers/float.h
cfe/trunk/test/Headers/float.c

Modified: cfe/trunk/lib/Headers/float.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/float.h?rev=353970&r1=353969&r2=353970&view=diff
==
--- cfe/trunk/lib/Headers/float.h (original)
+++ cfe/trunk/lib/Headers/float.h Wed Feb 13 11:08:01 2019
@@ -51,7 +51,7 @@
 #  undef FLT_MANT_DIG
 #  undef DBL_MANT_DIG
 #  undef LDBL_MANT_DIG
-#  if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__)
+#  if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || __cplusplus 
>= 201103L
 #undef DECIMAL_DIG
 #  endif
 #  undef FLT_DIG
@@ -78,7 +78,7 @@
 #  undef FLT_MIN
 #  undef DBL_MIN
 #  undef LDBL_MIN
-#  if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__)
+#  if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus 
>= 201103L
 #undef FLT_TRUE_MIN
 #undef DBL_TRUE_MIN
 #undef LDBL_TRUE_MIN
@@ -101,7 +101,7 @@
 #define DBL_MANT_DIG __DBL_MANT_DIG__
 #define LDBL_MANT_DIG __LDBL_MANT_DIG__
 
-#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__)
+#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
 #  define DECIMAL_DIG __DECIMAL_DIG__
 #endif
 
@@ -137,7 +137,7 @@
 #define DBL_MIN __DBL_MIN__
 #define LDBL_MIN __LDBL_MIN__
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__)
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
 #  define FLT_TRUE_MIN __FLT_DENORM_MIN__
 #  define DBL_TRUE_MIN __DBL_DENORM_MIN__
 #  define LDBL_TRUE_MIN __LDBL_DENORM_MIN__

Modified: cfe/trunk/test/Headers/float.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/float.c?rev=353970&r1=353969&r2=353970&view=diff
==
--- cfe/trunk/test/Headers/float.c (original)
+++ cfe/trunk/test/Headers/float.c Wed Feb 13 11:08:01 2019
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -ffreestanding %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -ffreestanding %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c11 -ffreestanding %s
+// RUN: %clang_cc1 -fsyntax-only -verify -xc++ -std=c++11 -ffreestanding %s
+// RUN: %clang_cc1 -fsyntax-only -verify -xc++ -std=c++14 -ffreestanding %s
+// RUN: %clang_cc1 -fsyntax-only -verify -xc++ -std=c++17 -ffreestanding %s
 // expected-no-diagnostics
 
 /* Basic floating point conformance checks against:
@@ -11,7 +14,7 @@
 /*
 C11,5.2.4.2.2p11,   pp. 30
 C99,5.2.4.2.2p9,pp. 25
-C89,2.2.4.2 
+C89,2.2.4.2
 */
 #include 
 
@@ -42,7 +45,7 @@
 #endif
 
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__)
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
 #ifndef FLT_DECIMAL_DIG
 #error "Mandatory macro FLT_DECIMAL_DIG is missing."
 #elif   FLT_DECIMAL_DIG < 6
@@ -98,7 +101,7 @@
 #endif
 
 
-#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__)
+#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
 #ifndef DECIMAL_DIG
 #error "Mandatory macro DECIMAL_DIG is missing."
 #elif   DECIMAL_DIG < 10
@@ -212,13 +215,13 @@ _Static_assert(FLT_MANT_DIG == __FLT_MAN
 _Static_assert(DBL_MANT_DIG == __DBL_MANT_DIG__, "");
 _Static_assert(LDBL_MANT_DIG == __LDBL_MANT_DIG__, "");
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__)
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
 _Static_assert(FLT_DECIMAL_DIG == __FLT_DECIMAL_DIG__, "");
 _Static_assert(DBL_DECIMAL_DIG == __DBL_DECIMAL_DIG__, "");
 _Static_assert(LDBL_DECIMAL_DIG == __LDBL_DECIMAL_DIG__, "");
 #endif
 
-#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__)
+#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
 _Static_assert(DECIMAL_DIG == __DECIMAL_DIG__, "");
 #endif
 


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

[PATCH] D58149: [clang] Make sure C99/C11 features in are provided in C++11

[Louis Dionne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353970: [clang] Make sure C99/C11 features in 
 are provided in C++11 (authored by ldionne, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58149?vs=186544&id=186703#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58149

Files:
  lib/Headers/float.h
  test/Headers/float.c


Index: lib/Headers/float.h
===
--- lib/Headers/float.h
+++ lib/Headers/float.h
@@ -51,7 +51,7 @@
 #  undef FLT_MANT_DIG
 #  undef DBL_MANT_DIG
 #  undef LDBL_MANT_DIG
-#  if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__)
+#  if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || __cplusplus 
>= 201103L
 #undef DECIMAL_DIG
 #  endif
 #  undef FLT_DIG
@@ -78,7 +78,7 @@
 #  undef FLT_MIN
 #  undef DBL_MIN
 #  undef LDBL_MIN
-#  if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__)
+#  if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus 
>= 201103L
 #undef FLT_TRUE_MIN
 #undef DBL_TRUE_MIN
 #undef LDBL_TRUE_MIN
@@ -101,7 +101,7 @@
 #define DBL_MANT_DIG __DBL_MANT_DIG__
 #define LDBL_MANT_DIG __LDBL_MANT_DIG__
 
-#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__)
+#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
 #  define DECIMAL_DIG __DECIMAL_DIG__
 #endif
 
@@ -137,7 +137,7 @@
 #define DBL_MIN __DBL_MIN__
 #define LDBL_MIN __LDBL_MIN__
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__)
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
 #  define FLT_TRUE_MIN __FLT_DENORM_MIN__
 #  define DBL_TRUE_MIN __DBL_DENORM_MIN__
 #  define LDBL_TRUE_MIN __LDBL_DENORM_MIN__
Index: test/Headers/float.c
===
--- test/Headers/float.c
+++ test/Headers/float.c
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -ffreestanding %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -ffreestanding %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c11 -ffreestanding %s
+// RUN: %clang_cc1 -fsyntax-only -verify -xc++ -std=c++11 -ffreestanding %s
+// RUN: %clang_cc1 -fsyntax-only -verify -xc++ -std=c++14 -ffreestanding %s
+// RUN: %clang_cc1 -fsyntax-only -verify -xc++ -std=c++17 -ffreestanding %s
 // expected-no-diagnostics
 
 /* Basic floating point conformance checks against:
@@ -11,7 +14,7 @@
 /*
 C11,5.2.4.2.2p11,   pp. 30
 C99,5.2.4.2.2p9,pp. 25
-C89,2.2.4.2 
+C89,2.2.4.2
 */
 #include 
 
@@ -42,7 +45,7 @@
 #endif
 
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__)
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
 #ifndef FLT_DECIMAL_DIG
 #error "Mandatory macro FLT_DECIMAL_DIG is missing."
 #elif   FLT_DECIMAL_DIG < 6
@@ -98,7 +101,7 @@
 #endif
 
 
-#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__)
+#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
 #ifndef DECIMAL_DIG
 #error "Mandatory macro DECIMAL_DIG is missing."
 #elif   DECIMAL_DIG < 10
@@ -212,13 +215,13 @@
 _Static_assert(DBL_MANT_DIG == __DBL_MANT_DIG__, "");
 _Static_assert(LDBL_MANT_DIG == __LDBL_MANT_DIG__, "");
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__)
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
 _Static_assert(FLT_DECIMAL_DIG == __FLT_DECIMAL_DIG__, "");
 _Static_assert(DBL_DECIMAL_DIG == __DBL_DECIMAL_DIG__, "");
 _Static_assert(LDBL_DECIMAL_DIG == __LDBL_DECIMAL_DIG__, "");
 #endif
 
-#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__)
+#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
 _Static_assert(DECIMAL_DIG == __DECIMAL_DIG__, "");
 #endif
 


Index: lib/Headers/float.h
===
--- lib/Headers/float.h
+++ lib/Headers/float.h
@@ -51,7 +51,7 @@
 #  undef FLT_MANT_DIG
 #  undef DBL_MANT_DIG
 #  undef LDBL_MANT_DIG
-#  if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__)
+#  if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || __cplusplus >= 201103L
 #undef DECIMAL_DIG
 #  endif
 #  undef FLT_DIG
@@ -78,7 +78,7 @@
 #  undef FLT_MIN
 #  undef DBL_MIN
 #  undef LDBL_MIN
-#  if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__)
+#  if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 201103L
 #undef FLT_TRUE_MIN
 #undef DBL_TRUE_MIN
 #undef LDBL_TRUE_MIN
@@ -101,7 +101,7 @@
 #define DBL_MANT_DIG __DBL_MANT_DIG__
 #define LDBL_MANT_DIG __LDBL_MANT_DIG__
 
-#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__)
+#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || __cplusplus >= 

[PATCH] D58163: [CUDA][HIP] Use device side kernel and variable names when registering them

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGCUDANV.cpp:412
+  for (auto &&I : EmittedKernels) {
+llvm::Constant *KernelName = makeConstantString(I.DeviceSideName);
 llvm::Constant *NullPtr = llvm::ConstantPointerNull::get(VoidPtrTy);

tra wrote:
> Can we just call `getDeviceSideName()` here ? Mangling the name early and 
> carrying it around does not seem to buy us anything.
getDeviceSideName() need to know Decl of the function. If we want to call it 
here, we have to carry Decl of the functions around.


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

https://reviews.llvm.org/D58163



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

[PATCH] D58120: [Builtins] Treat `bcmp` as a builtin.

[James Y Knight via Phabricator via cfe-commits]

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

Looks reasonable to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58120



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

[PATCH] D58163: [CUDA][HIP] Use device side kernel and variable names when registering them

[Artem Belevich via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGCUDANV.cpp:412
+  for (auto &&I : EmittedKernels) {
+llvm::Constant *KernelName = makeConstantString(I.DeviceSideName);
 llvm::Constant *NullPtr = llvm::ConstantPointerNull::get(VoidPtrTy);

yaxunl wrote:
> tra wrote:
> > Can we just call `getDeviceSideName()` here ? Mangling the name early and 
> > carrying it around does not seem to buy us anything.
> getDeviceSideName() need to know Decl of the function. If we want to call it 
> here, we have to carry Decl of the functions around.
I see. We appear to get both the Decl and llvm::function from CodeGenFunction.  
Perhaps we can make `EmittedKernels` a vector.

It's just a nit. I'll leave it up to you.


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

https://reviews.llvm.org/D58163



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

[PATCH] D58163: [CUDA][HIP] Use device side kernel and variable names when registering them

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGCUDANV.cpp:412
+  for (auto &&I : EmittedKernels) {
+llvm::Constant *KernelName = makeConstantString(I.DeviceSideName);
 llvm::Constant *NullPtr = llvm::ConstantPointerNull::get(VoidPtrTy);

tra wrote:
> yaxunl wrote:
> > tra wrote:
> > > Can we just call `getDeviceSideName()` here ? Mangling the name early and 
> > > carrying it around does not seem to buy us anything.
> > getDeviceSideName() need to know Decl of the function. If we want to call 
> > it here, we have to carry Decl of the functions around.
> I see. We appear to get both the Decl and llvm::function from 
> CodeGenFunction.  
> Perhaps we can make `EmittedKernels` a vector.
> 
> It's just a nit. I'll leave it up to you.
Carrying Decl around is better than carrying string around since it can save 
some space. Will do it.


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

https://reviews.llvm.org/D58163



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

[PATCH] D54978: Move the SMT API to LLVM

[Brian Rzycki via Phabricator via cfe-commits]

brzycki added a comment.

In D54978#1395562 , @mikhail.ramalho 
wrote:

> Hi @brzycki, but isn't it exactly what we want? I mean, if we try to 
> cross-compile and it fails for any reason (non-native library, wrong 
> version), then Z3_FOUND shouldn't be set.


I'm not sure, cross-compilation is tricky to get right.

> I just finished a small patch that checks the version as a run-time property. 
> Please, let me know your thoughts.
>  If you agree with this approach I'll create a separate revision for it (it 
> seems to work on ubuntu and fedora for me).

I'd be interested in seeing it and I'll happily provide feedback. I'm not sure 
if this method is better or worse than regex parsing of headers that @ddcc 
suggested.

No matter what algorithm chosen I'd strongly prefer the case when we are unable 
to ascertain the correct `Z3_VERSION_STRING` we should conservatively set this 
to `"0.0.0"` with a warning to inform the users early on something is strange 
with the version of Z3 being tested.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D58163: [CUDA][HIP] Use device side kernel and variable names when registering them

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 186711.
yaxunl added a comment.

Revised by Artem's comments.


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

https://reviews.llvm.org/D58163

Files:
  include/clang/AST/ASTContext.h
  lib/AST/ASTContext.cpp
  lib/CodeGen/CGCUDANV.cpp
  lib/CodeGen/CGCUDARuntime.h
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGenCUDA/device-stub.cu

Index: test/CodeGenCUDA/device-stub.cu
===
--- test/CodeGenCUDA/device-stub.cu
+++ test/CodeGenCUDA/device-stub.cu
@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -target-sdk-version=8.0 -fcuda-include-gpubinary %t -o - \
 // RUN:   | FileCheck -allow-deprecated-dag-overlap %s \
-// RUN:   --check-prefixes=ALL,NORDC,CUDA,CUDANORDC,CUDA-OLD
+// RUN:   --check-prefixes=ALL,LNX,NORDC,CUDA,CUDANORDC,CUDA-OLD
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -target-sdk-version=8.0  -fcuda-include-gpubinary %t \
 // RUN: -o - -DNOGLOBALS \
@@ -12,7 +12,7 @@
 // RUN: -target-sdk-version=8.0 -fgpu-rdc -fcuda-include-gpubinary %t \
 // RUN: -o - \
 // RUN:   | FileCheck -allow-deprecated-dag-overlap %s \
-// RUN:   --check-prefixes=ALL,RDC,CUDA,CUDARDC,CUDA-OLD
+// RUN:   --check-prefixes=ALL,LNX,RDC,CUDA,CUDARDC,CUDA-OLD
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -target-sdk-version=8.0 -o - \
 // RUN:   | FileCheck -allow-deprecated-dag-overlap %s -check-prefix=NOGPUBIN
@@ -20,7 +20,7 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s   \
 // RUN: -target-sdk-version=9.2 -fcuda-include-gpubinary %t -o - \
 // RUN:   | FileCheck %s -allow-deprecated-dag-overlap \
-// RUN:   --check-prefixes=ALL,NORDC,CUDA,CUDANORDC,CUDA-NEW
+// RUN:   --check-prefixes=ALL,LNX,NORDC,CUDA,CUDANORDC,CUDA-NEW
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -target-sdk-version=9.2 -fcuda-include-gpubinary %t -o -  -DNOGLOBALS \
 // RUN:   | FileCheck -allow-deprecated-dag-overlap %s \
@@ -28,56 +28,70 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -target-sdk-version=9.2 -fgpu-rdc -fcuda-include-gpubinary %t -o - \
 // RUN:   | FileCheck %s -allow-deprecated-dag-overlap \
-// RUN:   --check-prefixes=ALL,RDC,CUDA,CUDARDC,CUDA_NEW
+// RUN:   --check-prefixes=ALL,LNX,RDC,CUDA,CUDARDC,CUDA_NEW
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -target-sdk-version=9.2 -o - \
 // RUN:   | FileCheck -allow-deprecated-dag-overlap %s -check-prefix=NOGPUBIN
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -fcuda-include-gpubinary %t -o - -x hip\
-// RUN:   | FileCheck -allow-deprecated-dag-overlap %s --check-prefixes=ALL,NORDC,HIP,HIPEF
+// RUN:   | FileCheck -allow-deprecated-dag-overlap %s --check-prefixes=ALL,LNX,NORDC,HIP,HIPEF
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -fcuda-include-gpubinary %t -o -  -DNOGLOBALS -x hip \
 // RUN:   | FileCheck -allow-deprecated-dag-overlap %s -check-prefixes=NOGLOBALS,HIPNOGLOBALS
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -fgpu-rdc -fcuda-include-gpubinary %t -o - -x hip \
-// RUN:   | FileCheck -allow-deprecated-dag-overlap %s --check-prefixes=ALL,NORDC,HIP,HIPEF
+// RUN:   | FileCheck -allow-deprecated-dag-overlap %s --check-prefixes=ALL,LNX,NORDC,HIP,HIPEF
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - -x hip\
-// RUN:   | FileCheck -allow-deprecated-dag-overlap %s -check-prefixes=ALL,NORDC,HIP,HIPNEF
+// RUN:   | FileCheck -allow-deprecated-dag-overlap %s -check-prefixes=ALL,LNX,NORDC,HIP,HIPNEF
+
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -aux-triple amdgcn -emit-llvm %s \
+// RUN: -fcuda-include-gpubinary %t -o - -x hip\
+// RUN:   | FileCheck -allow-deprecated-dag-overlap %s --check-prefixes=ALL,WIN
 
 #include "Inputs/cuda.h"
 
 #ifndef NOGLOBALS
-// ALL-DAG: @device_var = internal global i32
+// LNX-DAG: @device_var = internal global i32
+// WIN-DAG: @"?device_var@@3HA" = internal global i32
 __device__ int device_var;
 
-// ALL-DAG: @constant_var = internal global i32
+// LNX-DAG: @constant_var = internal global i32
+// WIN-DAG: @"?constant_var@@3HA" = internal global i32
 __constant__ int constant_var;
 
-// ALL-DAG: @shared_var = internal global i32
+// LNX-DAG: @shared_var = internal global i32
+// WIN-DAG: @"?shared_var@@3HA" = internal global i32
 __shared__ int shared_var;
 
 // Make sure host globals don't get internalized...
-// ALL-DAG: @host_var = global i32
+// LNX-DAG: @host_var = global i32
+// WIN-DAG: @"?host_var@@3HA" = dso_local global i32
 int host_var;
 // ... and that extern vars remain external.
-// ALL-DAG: @ext_host_var = external global i32
+// LNX-DAG: @ext_host_var = external global i32
+// WIN-DAG: @"?ext_host_var@@3HA" = external dso_local global i32
 extern int ext_host_

[PATCH] D58163: [CUDA][HIP] Use device side kernel and variable names when registering them

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added a comment.

Thank you. LGTM.


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

https://reviews.llvm.org/D58163



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

[PATCH] D58157: Stop enabling clang-tools-extra automatically when clang is in LLVM_ENABLE_PROJECTS

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D58157#1395762 , @mehdi_amini wrote:

> In D58157#1395716 , @rnk wrote:
>
> > I think we have consensus,
>
>
> Based on three comments in a revision? Seems strange to me.
>  I don't really care about this, so do whatever you want, but I would expect 
> that "consensus" means an actual wider discussion (i.e. llvm-dev + cfe-dev).


Well, Sam's comment matters, since he works in the codebase in question. I also 
think I misread your first comment as being more positive on this. My 
(incorrect) interpretation was, "we were going to push clang-tools-extra under 
clang as part of the monorepo reorg, which is why we added this special case in 
cmake". And, given that we didn't do that reorganization and nobody intends to 
do it, it seems like the cmake should mirror the current code structure.

I'm not trying to approve things under the radar, I just want to expedite 
things without creating unneeded extra process. And, this change really just 
keeps us at parity with what we had with svn. We can always revisit the 
decision to merge the clang tools into clang. This particular change just gives 
us more options, today.


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

https://reviews.llvm.org/D58157



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

[PATCH] D58152: [Sema] Delay checking whether objc_designated_initializer is being applied to an init method

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7258-7267
+  // Do this check after processing D's attributes because the attribute
+  // objc_method_family can change whether the given method is in the init
+  // family, and it can be applied after objc_designated_initializer. This is a
+  // bit of a hack, but we need it to be compatible with versions of clang that
+  // processed the attribute list in the wrong order.
+  if (D->hasAttr() &&
+  cast(D)->getMethodFamily() != OMF_init) {

aaron.ballman wrote:
> Do you also have to handle redeclaration merging, or is that not a thing for 
> ObjC method declarations?
> 
> I'm wondering if this is better handled with the `mergeFooAttr()` pattern 
> from `Sema::mergeDeclAttribute()`?
You mean delaying the check until we see the method implementation? It seems 
like that would only enable the following:

```
@interface X
-(instancetype)meth __attribute__((objc_designated_initializer));
@end
@implementation X
-(instancetype)meth __attribute__((objc_method_family(init))) {}
@end
```

We never supported this, so there isn't any regression here. I don't even think 
that this makes sense, the `objc_method_family` attribute should really go in 
the @interface. It also means that we'd only diagnose this in the TU that 
contains the `@implementation`.


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

https://reviews.llvm.org/D58152



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