[PATCH] D60748: Fix i386 struct and union parameter alignment

2019-09-29 Thread LiuChen via Phabricator via cfe-commits 
LiuChen3 added a comment.

In D60748#1681178 , @kib wrote:

> In fact, can we have an option controlling this ?  Does it have anything to 
> do with -malign-data gcc switch ?
>
> We do want to be able to optionally generate code ABI-compatible with modern 
> gcc, per user discretion.


I found -malign-data option only affects data alignment in data segment. 
-malign-data has three options:  “compat”,“ abi” and “cacheline”.  The default 
in GCC is ”compat,“ and clang’s behavior is consistent with "abi". 
And the data alignment on stack and parameters Passing on stack is not 
affected.  This patch only affects the alignment of passing parameter.
Should we add an option just like -malign-data?


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

https://reviews.llvm.org/D60748



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

r373162 - [cxx_status] Mark P0784R7 as partially complete.

2019-09-29 Thread Richard Smith via cfe-commits 
Author: rsmith
Date: Sun Sep 29 00:16:13 2019
New Revision: 373162

URL: http://llvm.org/viewvc/llvm-project?rev=373162&view=rev
Log:
[cxx_status] Mark P0784R7 as partially complete.

Modified:
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/www/cxx_status.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_status.html?rev=373162&r1=373161&r2=373162&view=diff
==
--- cfe/trunk/www/cxx_status.html (original)
+++ cfe/trunk/www/cxx_status.html Sun Sep 29 00:16:13 2019
@@ -1010,7 +1010,7 @@ as the draft C++2a standard evolves.
   
   
 http://wg21.link/p0784r7";>P0784R7
-No
+Partial
   
 
   Prohibit aggregates with user-declared constructors


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

[PATCH] D68072: [clang-format] Reference qualifiers in member templates causing extra indentation.

2019-09-29 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

LG


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

https://reviews.llvm.org/D68072



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

[PATCH] D68072: [clang-format] Reference qualifiers in member templates causing extra indentation.

2019-09-29 Thread Andreas Wass via Phabricator via cfe-commits 
AndWass marked 2 inline comments as done.
AndWass added a comment.

Thanks! @MyDeveloperDay could you commit as well? Don't have access for that yet


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

https://reviews.llvm.org/D68072



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

r373165 - [clang-format] Reference qualifiers in member templates causing extra indentation.

2019-09-29 Thread Paul Hoad via cfe-commits 
Author: paulhoad
Date: Sun Sep 29 06:45:38 2019
New Revision: 373165

URL: http://llvm.org/viewvc/llvm-project?rev=373165&view=rev
Log:
[clang-format] Reference qualifiers in member templates causing extra 
indentation.

Summary:
The following code

```
struct f {
  template 
  void bar() && noexcept {}
};
```

will be formatted to the following with LLVM style, and
`AlwaysBreakTemplateDeclarations: Yes`

```
struct f {
  template 
  void bar() && noexcept {}
};
```

The indentation of the `void bar()` line is wrong.

Reviewers: klimek, owenpan, krasimir, timwoj, MyDeveloperDay

Reviewed By: klimek, MyDeveloperDay

Subscribers: MyDeveloperDay, ilya-biryukov, llvm-commits, cfe-commits

Patch By: AndWass

Tags: #clang-format, #clang, #llvm

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=373165&r1=373164&r2=373165&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Sun Sep 29 06:45:38 2019
@@ -65,7 +65,7 @@ public:
   AnnotatingParser(const FormatStyle &Style, AnnotatedLine &Line,
const AdditionalKeywords &Keywords)
   : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false),
-Keywords(Keywords) {
+TrailingReturnFound(false), Keywords(Keywords) {
 Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
 resetTokenMetadata(CurrentToken);
   }
@@ -1389,7 +1389,11 @@ private:
 } else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
Current.NestingLevel == 0) {
   Current.Type = TT_TrailingReturnArrow;
-} else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
+  TrailingReturnFound = true;
+} else if (Current.is(tok::star) ||
+   (Current.isOneOf(tok::amp, tok::ampamp) &&
+(Current.NestingLevel != 0 || !Line.MightBeFunctionDecl ||
+ TrailingReturnFound))) {
   Current.Type = determineStarAmpUsage(Current,
Contexts.back().CanBeExpression &&
Contexts.back().IsExpression,
@@ -1412,6 +1416,8 @@ private:
 Current.Type = TT_ConditionalExpr;
   }
 } else if (Current.isBinaryOperator() &&
+   !(Line.MightBeFunctionDecl && Current.NestingLevel == 0 &&
+ Current.isOneOf(tok::amp, tok::ampamp)) &&
(!Current.Previous || Current.Previous->isNot(tok::l_square)) &&
(!Current.is(tok::greater) &&
 Style.Language != FormatStyle::LK_TextProto)) {
@@ -1486,10 +1492,12 @@ private:
   // colon after this, this is the only place which annotates the 
identifier
   // as a selector.)
   Current.Type = TT_SelectorName;
-} else if (Current.isOneOf(tok::identifier, tok::kw_const) &&
+} else if (Current.isOneOf(tok::identifier, tok::kw_const, tok::amp,
+   tok::ampamp) &&
Current.Previous &&
!Current.Previous->isOneOf(tok::equal, tok::at) &&
-   Line.MightBeFunctionDecl && Contexts.size() == 1) {
+   Line.MightBeFunctionDecl && !TrailingReturnFound &&
+   Contexts.size() == 1) {
   // Line.MightBeFunctionDecl can only be true after the parentheses of a
   // function declaration have been found.
   Current.Type = TT_TrailingAnnotation;
@@ -1767,6 +1775,7 @@ private:
   AnnotatedLine &Line;
   FormatToken *CurrentToken;
   bool AutoFound;
+  bool TrailingReturnFound;
   const AdditionalKeywords &Keywords;
 
   // Set of "<" tokens that do not open a template parameter list. If 
parseAngle
@@ -2680,6 +2689,14 @@ bool TokenAnnotator::spaceRequiredBetwee
   Right.MatchingParen->endsSequence(TT_DictLiteral, tok::at))
 // Objective-C dictionary literal -> no space before closing brace.
 return false;
+  if (Right.Type == TT_TrailingAnnotation &&
+  Right.isOneOf(tok::amp, tok::ampamp) &&
+  Left.isOneOf(tok::kw_const, tok::kw_volatile) &&
+  (!Right.Next || Right.Next->is(tok::semi)))
+// Match const and volatile ref-qualifiers without any additional
+// qualifiers such as
+// void Fn() const &;
+return Style.PointerAlignment != FormatStyle::PAS_Left;
   return true;
 }
 

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=373165&r1=373164&r2=373165&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Sun Sep 29 06:4

[PATCH] D68072: [clang-format] Reference qualifiers in member templates causing extra indentation.

2019-09-29 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373165: [clang-format] Reference qualifiers in member 
templates causing extra… (authored by paulhoad, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D68072?vs=99&id=222322#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68072

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -65,7 +65,7 @@
   AnnotatingParser(const FormatStyle &Style, AnnotatedLine &Line,
const AdditionalKeywords &Keywords)
   : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false),
-Keywords(Keywords) {
+TrailingReturnFound(false), Keywords(Keywords) {
 Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
 resetTokenMetadata(CurrentToken);
   }
@@ -1389,7 +1389,11 @@
 } else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
Current.NestingLevel == 0) {
   Current.Type = TT_TrailingReturnArrow;
-} else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
+  TrailingReturnFound = true;
+} else if (Current.is(tok::star) ||
+   (Current.isOneOf(tok::amp, tok::ampamp) &&
+(Current.NestingLevel != 0 || !Line.MightBeFunctionDecl ||
+ TrailingReturnFound))) {
   Current.Type = determineStarAmpUsage(Current,
Contexts.back().CanBeExpression &&
Contexts.back().IsExpression,
@@ -1412,6 +1416,8 @@
 Current.Type = TT_ConditionalExpr;
   }
 } else if (Current.isBinaryOperator() &&
+   !(Line.MightBeFunctionDecl && Current.NestingLevel == 0 &&
+ Current.isOneOf(tok::amp, tok::ampamp)) &&
(!Current.Previous || Current.Previous->isNot(tok::l_square)) &&
(!Current.is(tok::greater) &&
 Style.Language != FormatStyle::LK_TextProto)) {
@@ -1486,10 +1492,12 @@
   // colon after this, this is the only place which annotates the identifier
   // as a selector.)
   Current.Type = TT_SelectorName;
-} else if (Current.isOneOf(tok::identifier, tok::kw_const) &&
+} else if (Current.isOneOf(tok::identifier, tok::kw_const, tok::amp,
+   tok::ampamp) &&
Current.Previous &&
!Current.Previous->isOneOf(tok::equal, tok::at) &&
-   Line.MightBeFunctionDecl && Contexts.size() == 1) {
+   Line.MightBeFunctionDecl && !TrailingReturnFound &&
+   Contexts.size() == 1) {
   // Line.MightBeFunctionDecl can only be true after the parentheses of a
   // function declaration have been found.
   Current.Type = TT_TrailingAnnotation;
@@ -1767,6 +1775,7 @@
   AnnotatedLine &Line;
   FormatToken *CurrentToken;
   bool AutoFound;
+  bool TrailingReturnFound;
   const AdditionalKeywords &Keywords;
 
   // Set of "<" tokens that do not open a template parameter list. If parseAngle
@@ -2680,6 +2689,14 @@
   Right.MatchingParen->endsSequence(TT_DictLiteral, tok::at))
 // Objective-C dictionary literal -> no space before closing brace.
 return false;
+  if (Right.Type == TT_TrailingAnnotation &&
+  Right.isOneOf(tok::amp, tok::ampamp) &&
+  Left.isOneOf(tok::kw_const, tok::kw_volatile) &&
+  (!Right.Next || Right.Next->is(tok::semi)))
+// Match const and volatile ref-qualifiers without any additional
+// qualifiers such as
+// void Fn() const &;
+return Style.PointerAlignment != FormatStyle::PAS_Left;
   return true;
 }
 
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -6901,6 +6901,74 @@
Spaces);
   verifyFormat("Deleted &operator=( const Deleted & ) &;", Spaces);
   verifyFormat("SomeType MemberFunction( const Deleted & ) &;", Spaces);
+
+  FormatStyle BreakTemplate = getLLVMStyle();
+  BreakTemplate.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo(const std::string &str) & noexcept {}\n"
+   "};",
+   BreakTemplate);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo(const std::string &str) && noexcept {}\n"
+   "};",
+   BreakTemplate);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo(const std::string &str) const & noexcep

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

2019-09-29 Thread Aditya Kumar via Phabricator via cfe-commits 
hiraditya added a comment.

it will be great to merge this patch.


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

https://reviews.llvm.org/D57265



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

[PATCH] D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

2019-09-29 Thread Kousik Kumar via Phabricator via cfe-commits 
kousikk created this revision.
kousikk added a reviewer: arphaman.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

It seems that when the CachingFileSystem is first given a file to open that is 
actually a directory, it incorrectly
caches that path to be errenous and throws an error when subsequently a 
directory open call is made for the same
path.
This change makes it so that we do NOT cache a path if it turns out we asked 
for a file when its a directory.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68193

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
  clang/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp


Index: clang/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.dir/foodir
+// RUN: rm -rf %t.cdb
+
+// RUN: mkdir -p %t.dir
+// RUN: mkdir -p %t.dir/foodir
+
+// RUN: cp %S/Inputs/header.h %t.dir/foodir/foodirheader.h
+// RUN: cp %s %t.dir/headerwithdirname_input.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/foodir %t.dir/Inputs/foodir
+// RUN: sed -e "s|DIR|%/t.dir|g" 
%S/Inputs/headerwithdirnamefollowedbyinclude.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#include 
+#include "foodir/foodirheader.h"
+
+// CHECK: headerwithdirname_input.o
+// CHECK-NEXT: headerwithdirname_input.cpp
+// CHECK-NEXT: Inputs{{/|\\}}foodir
Index: clang/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -c -IDIR -IInputs DIR/headerwithdirname_input.cpp",
+  "file": "DIR/headerwithdirname_input.cpp"
+}
+]
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -233,8 +233,15 @@
 CachedFileSystemEntry &CacheEntry = SharedCacheEntry.Value;
 
 if (!CacheEntry.isValid()) {
+  llvm::vfs::FileSystem &FS = getUnderlyingFS();
+  auto MaybeStatus = FS.status(Filename);
+
+  if (MaybeStatus && MaybeStatus->isDirectory())
+return llvm::ErrorOr>(
+  std::make_error_code(std::errc::is_a_directory));
+
   CacheEntry = CachedFileSystemEntry::createFileEntry(
-  Filename, getUnderlyingFS(), !KeepOriginalSource);
+  Filename, FS, !KeepOriginalSource);
 }
 
 Result = &CacheEntry;


Index: clang/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.dir/foodir
+// RUN: rm -rf %t.cdb
+
+// RUN: mkdir -p %t.dir
+// RUN: mkdir -p %t.dir/foodir
+
+// RUN: cp %S/Inputs/header.h %t.dir/foodir/foodirheader.h
+// RUN: cp %s %t.dir/headerwithdirname_input.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/foodir %t.dir/Inputs/foodir
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/headerwithdirnamefollowedbyinclude.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#include 
+#include "foodir/foodirheader.h"
+
+// CHECK: headerwithdirname_input.o
+// CHECK-NEXT: headerwithdirname_input.cpp
+// CHECK-NEXT: Inputs{{/|\\}}foodir
Index: clang/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -c -IDIR -IInputs DIR/headerwithdirname_input.cpp",
+  "file": "DIR/headerwithdirname_input.cpp"
+}
+]
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -233,8 +233,15 @@
 CachedFileSystemEntry &CacheEntry = SharedCacheEntry.Value;
 
 if (!CacheEntry.isValid()) {
+  llvm::vfs::FileSystem &FS = getUnderlyingFS();
+  auto MaybeStatus = FS.status(Filename);
+
+  if (MaybeStatus && MaybeStatus->isDirectory())
+return llvm::ErrorOr>(
+  std::make_error_code(std::errc::is_a_directory));
+
   C

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

2019-09-29 Thread Florian Hahn via Phabricator via cfe-commits 
fhahn added a comment.

This seems fine to me.

IIUC the only potential drawback with the old pass manager is that we 
potentially have to run the required passes unconditionally, even if we do not 
use them.  Vedant, did you have a chance to check the impact on overall compile 
time?




Comment at: clang/test/CodeGen/split-cold-code.c:69
+
+// SPLIT: "hot-cold-split"
+

Could you extend the scope of the check to include a bit more context, i.e. 
make sure we emit a function attribute attached to the correct function?



Comment at: llvm/test/Other/opt-Os-pipeline.ll:249
+; CHECK-NEXT: Hot Cold Splitting
+; CHECK-NEXT:   Unnamed pass: implement Pass::getPassName()
 ; CHECK-NEXT: FunctionPass Manager

Are we missing an implementation of getPassName in HotColdSplitting?


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

https://reviews.llvm.org/D57265



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

[PATCH] D53514: os_log: make buffer size an integer constant expression.

2019-09-29 Thread Aditya Kumar via Phabricator via cfe-commits 
hiraditya closed this revision.
hiraditya added a comment.
Herald added a project: clang.

closed in r345828, 314fbfa1c4c6665c54a220eefb10a6f23010a352 



Repository:
  rC Clang

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

https://reviews.llvm.org/D53514



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

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

2019-09-29 Thread Matthias Gehre via Phabricator via cfe-commits 
mgehre updated this revision to Diff 222331.
mgehre marked 6 inline comments as done.
mgehre added a comment.

- Implement review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68074

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
  clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-make-member-function-const.rst
  clang-tools-extra/test/clang-tidy/readability-make-member-function-const.cpp

Index: clang-tools-extra/test/clang-tidy/readability-make-member-function-const.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/readability-make-member-function-const.cpp
@@ -0,0 +1,321 @@
+// RUN: %check_clang_tidy %s readability-make-member-function-const %t
+
+struct Str {
+  void const_method() const;
+  void non_const_method();
+};
+
+namespace Diagnose {
+struct A;
+
+void free_const_use(const A *);
+void free_const_use(const A &);
+
+struct A {
+  int M;
+  const int ConstM;
+  struct {
+int M;
+  } Struct;
+  Str S;
+  Str Sref;
+
+  void already_const() const;
+
+  int read_field() {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'read_field' can be made const
+// CHECK-FIXES: {{^}}  int read_field() const {
+return M;
+  }
+
+  int read_struct_field() {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'read_struct_field' can be made const
+// CHECK-FIXES: {{^}}  int read_struct_field() const {
+return Struct.M;
+  }
+
+  int read_const_field() {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'read_const_field' can be made const
+// CHECK-FIXES: {{^}}  int read_const_field() const {
+return ConstM;
+  }
+
+  void call_const_member() {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'call_const_member' can be made const
+// CHECK-FIXES: {{^}}  void call_const_member() const {
+already_const();
+  }
+
+  void call_const_member_on_public_field() {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'call_const_member_on_public_field' can be made const
+// CHECK-FIXES: {{^}}  void call_const_member_on_public_field() const {
+S.const_method();
+  }
+
+  void call_const_member_on_public_field_ref() {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'call_const_member_on_public_field_ref' can be made const
+// CHECK-FIXES: {{^}}  void call_const_member_on_public_field_ref() const {
+Sref.const_method();
+  }
+
+  const Str &return_public_field_ref() {
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: method 'return_public_field_ref' can be made const
+// CHECK-FIXES: {{^}}  const Str &return_public_field_ref() const {
+return S;
+  }
+
+  const A *return_this_const() {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: method 'return_this_const' can be made const
+// CHECK-FIXES: {{^}}  const A *return_this_const() const {
+return this;
+  }
+
+  const A &return_this_const_ref() {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: method 'return_this_const_ref' can be made const
+// CHECK-FIXES: {{^}}  const A &return_this_const_ref() const {
+return *this;
+  }
+
+  void const_use() {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'const_use' can be made const
+// CHECK-FIXES: {{^}}  void const_use() const {
+free_const_use(this);
+  }
+
+  void const_use_ref() {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'const_use_ref' can be made const
+// CHECK-FIXES: {{^}}  void const_use_ref() const {
+free_const_use(*this);
+  }
+
+  auto trailingReturn() -> int {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'trailingReturn' can be made const
+// CHECK-FIXES: {{^}}  auto trailingReturn() const -> int {
+return M;
+  }
+
+  int volatileFunction() volatile {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'volatileFunction' can be made const
+// CHECK-FIXES: {{^}}  int volatileFunction() const volatile {
+return M;
+  }
+
+  int restrictFunction() __restrict {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'restrictFunction' can be made const
+// CHECK-FIXES: {{^}}  int restrictFunction() const __restrict {
+return M;
+  }
+
+  int refFunction() & {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'refFunction' can be made const
+// CHECK-FIXES: {{^}}  int refFunction() const & {
+return M;
+  }
+
+  void out_of_line_call_const();
+  // CHECK-FIXES: {{^}}  void out_of_line_call_const() const;
+};
+
+void A::out_of_line_call_const() {
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: method 'out_of_line_call_const' ca

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

2019-09-29 Thread Matthias Gehre via Phabricator via cfe-commits 
mgehre added a comment.

In D68074#1683770 , @lebedev.ri wrote:

> Awesome!
>  I believe i have asked this question in the convert-to-static diff - can 
> ExprMutAnalyzer be used here?


I believe in this case, we can get away with a simpler analysis. Because `this` 
is an prvalue,
there are less operations that can be done to it, and it seems easy to just 
list those here. Also, we want
to conservatively disallow "const" access to non-public member 
variables/functions, and I don't see
how this would elegantly integrate with using the ExprMutationAnalyzer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68074



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

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

2019-09-29 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

Since https://en.cppreference.com/w/cpp/language/rule_of_three says "almost 
always" (note "almost") I'd say this probably belongs more in clang-tidy 
territory – I'm guessing false positive rate is very high for this. Can you 
collect some true / false positive statistics on some large open-source project 
(chromium, firefox, open office, …)?

Since C++11 it's rule of 0/3/5, so this should probably handle the move 
ctor/assign op too.


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

https://reviews.llvm.org/D68185



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

[PATCH] D68076: [AArch64] Enable unwind tables by default for Gnu targets

2019-09-29 Thread Aditya Kumar via Phabricator via cfe-commits 
hiraditya added a comment.

Can we add a test to verify cfi instructions are present without debug flag.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68076



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

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

2019-09-29 Thread Dávid Bolvanský via Phabricator via cfe-commits 
xbolva00 abandoned this revision.
xbolva00 added a comment.

Yeah, if we want to check rule of 3, clang-tidy is better choice.


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

https://reviews.llvm.org/D68185



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

[PATCH] D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154)

2019-09-29 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge updated this revision to Diff 222333.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67901

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -51,6 +51,9 @@
   {HighlightingKind::StaticField, "StaticField"},
   {HighlightingKind::Method, "Method"},
   {HighlightingKind::StaticMethod, "StaticMethod"},
+  {HighlightingKind::Typedef, "Typedef"},
+  {HighlightingKind::DependentType, "DependentType"},
+  {HighlightingKind::DependentName, "DependentName"},
   {HighlightingKind::TemplateParameter, "TemplateParameter"},
   {HighlightingKind::Primitive, "Primitive"},
   {HighlightingKind::Macro, "Macro"}};
@@ -175,7 +178,7 @@
   }
   template
   struct $Class[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
-typename $TemplateParameter[[T]]::A* $Field[[D]];
+typename $TemplateParameter[[T]]::$DependentType[[A]]* $Field[[D]];
   };
   $Namespace[[abc]]::$Class[[A]]<$Primitive[[int]]> $Variable[[AA]];
   typedef $Namespace[[abc]]::$Class[[A]]<$Primitive[[int]]> $Class[[AAA]];
@@ -509,6 +512,55 @@
   $Typedef[[Pointer]], $Typedef[[LVReference]], $Typedef[[RVReference]],
   $Typedef[[Array]], $Typedef[[MemberPointer]]);
   };
+)cpp",
+  R"cpp(
+  template 
+  $Primitive[[void]] $Function[[foo]]($TemplateParameter[[T]] $Parameter[[P]]) {
+$DependentName[[bar]]($Parameter[[P]]);
+  }
+)cpp",
+  R"cpp(
+  class $Class[[A]] {
+template 
+$Primitive[[void]] $Method[[bar]]($TemplateParameter[[T]]);
+  };
+
+  template 
+  $Primitive[[void]] $Function[[foo]]($TemplateParameter[[U]] $Parameter[[P]]) {
+$Class[[A]]().$DependentName[[bar]]($Parameter[[P]]);
+  }
+)cpp",
+  R"cpp(
+  struct $Class[[A]] {
+template 
+static $Primitive[[void]] $StaticMethod[[foo]]($TemplateParameter[[T]]);
+  };
+
+  template 
+  struct $Class[[B]] {
+$Primitive[[void]] $Method[[bar]]() {
+  $Class[[A]]::$DependentName[[foo]]($TemplateParameter[[T]]());
+}
+  };
+)cpp",
+  R"cpp(
+  template 
+  $Primitive[[void]] $Function[[foo]](typename $TemplateParameter[[T]]::$DependentType[[Type]]
+= $TemplateParameter[[T]]::$DependentName[[val]]);
+)cpp",
+  R"cpp(
+  template 
+  $Primitive[[void]] $Function[[foo]]($TemplateParameter[[T]] $Parameter[[P]]) {
+$Parameter[[P]].$DependentName[[Field]];
+  }
+)cpp",
+  R"cpp(
+  template 
+  class $Class[[A]] {
+$Primitive[[int]] $Method[[foo]]() {
+  return $TemplateParameter[[T]]::$DependentName[[Field]];
+}
+  };
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -41,6 +41,12 @@
 # CHECK-NEXT:"entity.name.type.typedef.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
+# CHECK-NEXT:"entity.name.type.dependent.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CHECK-NEXT:"entity.name.other.dependent.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.namespace.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
@@ -61,7 +67,7 @@
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 0,
-# CHECK-NEXT:"tokens": "AAADAA4EAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADABAEAAEAAA=="
 # CHECK-NEXT:  }
 # CHECK-NEXT:],
 # CHECK-NEXT:"textDocument": {
@@ -76,11 +82,11 @@
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 0,
-# CHECK-NEXT:"tokens": "AAADAA4EAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADABAEAAEAAA=="
 # CHECK-NEXT:  }
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 1,
-# CHECK-NEXT:"tokens": "AAADAA4EAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADABAEAAEAAA=="
 # CHECK-NEXT:  }
 # CHECK-NEXT:],
 # CHECK-NEXT:"textDocument": {
@@ -95,7 +101,7 @@
 # CHECK-NEXT:"lines"

[PATCH] D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154)

2019-09-29 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge added a comment.

I do feel strongly that types and non-types should be highlighted differently, 
so the updated patch adds two new dependent highlightings, one for types and 
one for variables/functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67901



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

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

2019-09-29 Thread Jussi Pakkanen via Phabricator via cfe-commits 
jpakkane updated this revision to Diff 222335.
jpakkane added a comment.

Rebased against master to fix a merge conflict in release notes.


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

https://reviews.llvm.org/D64671

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp

Index: clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp
@@ -0,0 +1,80 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t
+
+// Ensure that function declarations are not changed.
+void some_func(int x, double d, bool b, const char *p);
+
+// Ensure that function arguments are not changed
+int identity_function(int x) {
+  return x;
+}
+
+int do_not_modify_me;
+
+static int should_not_be_initialized;
+extern int should_not_be_initialized2;
+
+typedef struct {
+  int unaltered1;
+  int unaltered2;
+} UnusedStruct;
+
+typedef int my_int_type;
+#define MACRO_INT int
+#define FULL_DECLARATION() int macrodecl;
+
+template 
+void template_test_function() {
+  T t;
+  int uninitialized;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'uninitialized' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int uninitialized = 0;{{$}}
+}
+
+void init_unit_tests() {
+  int x;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'x' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int x = 0;{{$}}
+  my_int_type myint;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'myint' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  my_int_type myint = 0;{{$}}
+
+  MACRO_INT macroint;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'macroint' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  MACRO_INT macroint = 0;{{$}}
+  FULL_DECLARATION();
+
+  int x0 = 1, x1, x2 = 2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'x1' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int x0 = 1, x1 = 0, x2 = 2;{{$}}
+  int y0, y1 = 1, y2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'y0' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: variable 'y2' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int y0 = 0, y1 = 1, y2 = 0;{{$}}
+  int hasval = 42;
+
+  float f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'f' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  float f = NAN;{{$}}
+  float fval = 85.0;
+  double d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: variable 'd' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  double d = NAN;{{$}}
+  double dval = 99.0;
+
+  bool b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable 'b' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  bool b = 0;{{$}}
+  bool bval = true;
+
+  const char *ptr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'ptr' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  const char *ptr = nullptr;{{$}}
+  const char *ptrval = "a string";
+
+  UnusedStruct u;
+
+  static int does_not_need_an_initializer;
+  extern int does_not_need_an_initializer2;
+  int parens(42);
+  int braces{42};
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -194,6 +194,7 @@
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) 
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-explicit-virtual-functions (redirects to modernize-use-override) 
+   cppcoreguidelines-init-variables
cppcoreguidelines-interfaces-global-init
cppcoreguidelines-macro-usage
cppcoreguidelines-narrowing-conversions
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst
@@ -0,0 +1,51 @@
+.. title:: clang-tidy - cppcoreguidelines-init-variables
+
+cppcoreguideli

r373173 - Don't crash if a variable with a constexpr destructor has a

2019-09-29 Thread Richard Smith via cfe-commits 
Author: rsmith
Date: Sun Sep 29 13:30:13 2019
New Revision: 373173

URL: http://llvm.org/viewvc/llvm-project?rev=373173&view=rev
Log:
Don't crash if a variable with a constexpr destructor has a
value-dependent initializer.

Modified:
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=373173&r1=373172&r2=373173&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Sun Sep 29 13:30:13 2019
@@ -13401,7 +13401,8 @@ void Sema::FinalizeVarWithDestructor(Var
 
   // If the destructor is constexpr, check whether the variable has constant
   // destruction now.
-  if (Destructor->isConstexpr() && VD->evaluateValue()) {
+  if (Destructor->isConstexpr() && VD->getInit() &&
+  !VD->getInit()->isValueDependent() && VD->evaluateValue()) {
 SmallVector Notes;
 if (!VD->evaluateDestruction(Notes) && VD->isConstexpr()) {
   Diag(VD->getLocation(),

Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp?rev=373173&r1=373172&r2=373173&view=diff
==
--- cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp Sun Sep 29 13:30:13 
2019
@@ -1269,3 +1269,12 @@ namespace temp_dtor {
   // FIXME: We could in prinicple accept this.
   constexpr const A &c = A{false}; // expected-error {{constant}} 
expected-note {{non-trivial destruction of lifetime-extended temporary}}
 }
+
+namespace value_dependent_init {
+  struct A {
+constexpr ~A() {}
+  };
+  template void f() {
+A a = T();
+  }
+}


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

[PATCH] D67161: [clang,ARM] Initial ACLE intrinsics for MVE.

2019-09-29 Thread Dave Green via Phabricator via cfe-commits 
dmgreen added a comment.

Sorry. I wasn't ignoring this (sort-of), I just knew that you were on holiday 
and this is a bit of a big one.

But I like this. I still have to take a deeper look into the main tablegen 
parts, but it looks very powerful.

I presume from here adding new intrinsics is mostly a case of adding to 
arm_mve.td (with perhaps some minor parts in defs and maybe some custom bits 
here and there).




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8529
   "argument should be a multiple of %0">;
+def err_argument_not_power_of_2 : Error<
+  "argument should be a power of 2">;

Do we have any/should we have some tests for these errors?



Comment at: clang/include/clang/Basic/arm_mve_defs.td:266
+// vidupq_wb_u16 -> vidupq_u16.
+def PNT_WB: PolymorphicNameType<0, "wb">;
+

These are not used yet, right? They are not meant for the vldr gathers, those 
don't have polymorphic names (if I'm reading this list of intrinsics right). 
These are for vidup's as the comment says?



Comment at: clang/test/CodeGen/arm-mve-intrinsics/scalar-shifts.c:2
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O3 -S -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -DPOLYMORPHIC -O3 -S -emit-llvm -o - %s | FileCheck %s

These tests all run -O3, the entire pass pipeline. I see quite a few tests in 
the same folder do the same thing, but in this case we will be adding quite a 
few tests. Random mid-end optimizations may well keep on altering them.

Is it possible to use -disable-O0-optnone and pipe them into opt -mem2reg? What 
would that do to the codegen, would it be a lot more verbose than it is now?

Also could/should they be using clang_cc1?



Comment at: clang/test/CodeGen/arm-mve-intrinsics/scalar-shifts.c:3
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O3 -S -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -DPOLYMORPHIC -O3 -S -emit-llvm -o - %s | FileCheck %s
+

POLYMORPHIC isn't used here. Same for vcvt below (Is there a polymorphic vcvt?)



Comment at: clang/test/CodeGen/arm-mve-intrinsics/vminvq.c:12
+//
+uint32_t test_vminvq_u32(uint32_t a, uint32x4_t b)
+{

Its probably worth trying to fill in tests for most types.



Comment at: clang/utils/TableGen/MveEmitter.cpp:82
+#if 0
+} // stop emacs from wanting to auto-indent everything to 2 spaces inside here
+#endif

Is this needed still? It seems like something that should be handled in 
clang-format/emacs directly.



Comment at: clang/utils/TableGen/MveEmitter.cpp:1403
+" *\n"
+" * Permission is hereby granted, free of charge, to any person "
+"obtaining a copy\n"

Clang format really made a mess of this, didn't it.

Is this is old license? Should it be updated to the new one. I imagine that 
these generated headers might well have been missed in the switchover.



Comment at: clang/utils/TableGen/MveEmitter.cpp:132
+// the pointee type.
+Pointer,
+  };

The gathers are really a Vector of Pointers, in a way. But in the intrinsics 
(and IR), they are just treated as a vector of ints, so I presume a new type is 
not needed? We may (but not yet) want to make use of the llvm masked gathers. 
We would have to add codegen support for them first though (which I don't think 
we have plans for in the near term).



Comment at: clang/utils/TableGen/MveEmitter.cpp:520
+  // time.
+  bool needs_visiting(unsigned Pass) {
+bool ToRet = Visited < Pass;

needsVisiting.

I would guess the same for other places that use snake_case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67161



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

[PATCH] D68148: [clang-tidy] Rename objc-avoid-spinlock check to darwin-avoid-spinlock

2019-09-29 Thread Stephane Moore via Phabricator via cfe-commits 
stephanemoore accepted this revision.
stephanemoore added a comment.
This revision is now accepted and ready to land.

I wasn't sure what the policy is for renaming checks so I dug into previous 
check renames. https://reviews.llvm.org/D43867 seems to indicate that it's 
acceptable for checks to be renamed with documentation in the release notes 
(though there is some discussion about the possibility of recognizing the old 
check name for a grace period). Based on the aforementioned precedent I don't 
have concerns with this proposed change.

LGTM 👌


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

https://reviews.llvm.org/D68148



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

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

2019-09-29 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

If you strongly feel that this is the right direction, go for it, you're code 
owner here :)

To me, this feels like a regression. This change has no benefit that I can see 
(at least none that anybody's asked for that I'm aware of – then again I read 
llvm's bugzilla less than you do), and the drawback that folks have to 
explicitly pass 
`/libpath:\path\to\clang\lib\clang\$changing_version\lib\windows` to the 
linker, which to me is a pretty poor experience. I'd prefer if we used this 
qualified path for all runtime libs, so that users would never have to use this 
libpath. (We currently use a qualified path for libprofile but not asan and the 
like iirc.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65543



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

[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

2019-09-29 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge updated this revision to Diff 222336.
nridge added a comment.
Herald added subscribers: usaxena95, mgrang.

Reformulate as an extension to semantic highlighting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test


Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -50,6 +50,9 @@
 # CHECK-NEXT:"storage.type.primitive.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
+# CHECK-NEXT:"meta.disabled"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.function.preprocessor.cpp"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -40,6 +40,7 @@
   Namespace,
   TemplateParameter,
   Primitive,
+  InactivePreprocessorBranch,
   Macro,
 
   LastKind = Macro
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -137,6 +137,29 @@
   // the end of the Tokens).
   TokRef = TokRef.drop_front(Conflicting.size());
 }
+// Add inactive highlighting tokens.
+const SourceManager &SM = AST.getSourceManager();
+for (const SourceRange &R :
+ AST.getPreprocessor().getPreprocessingRecord()->getSkippedRanges()) {
+  if (isInsideMainFile(R.getBegin(), SM)) {
+// Create one token for each line in the skipped range, so it works
+// with line-based diffing.
+Position Begin = sourceLocToPosition(SM, R.getBegin());
+Position End = sourceLocToPosition(SM, R.getEnd());
+assert(Begin.line <= End.line);
+for (int Line = Begin.line; Line <= End.line; ++Line) {
+  // Don't bother computing the offset for the end of the line, just 
use
+  // zero. The client will treat this highlighting kind specially, and
+  // highlight the entire line visually (i.e. not just to where the 
text
+  // on the line ends, but to the end of the screen).
+  NonConflicting.push_back(
+  {HighlightingKind::InactivePreprocessorBranch,
+   {Position{Line, 0}, Position{Line, 0}}});
+}
+  }
+}
+// Re-sort the tokens because that's what the diffing expects.
+llvm::sort(NonConflicting);
 return NonConflicting;
   }
 
@@ -345,6 +368,8 @@
 return OS << "TemplateParameter";
   case HighlightingKind::Primitive:
 return OS << "Primitive";
+  case HighlightingKind::InactivePreprocessorBranch:
+return OS << "InactivePreprocessorBranch";
   case HighlightingKind::Macro:
 return OS << "Macro";
   }
@@ -474,6 +499,8 @@
 return "entity.name.type.template.cpp";
   case HighlightingKind::Primitive:
 return "storage.type.primitive.cpp";
+  case HighlightingKind::InactivePreprocessorBranch:
+return "meta.disabled";
   case HighlightingKind::Macro:
 return "entity.name.function.preprocessor.cpp";
   }
Index: clang-tools-extra/clangd/Compiler.cpp
===
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -63,6 +63,7 @@
   // createInvocationFromCommandLine sets DisableFree.
   CI->getFrontendOpts().DisableFree = false;
   CI->getLangOpts()->CommentOpts.ParseAllComments = true;
+  CI->getPreprocessorOpts().DetailedRecord = true;
   return CI;
 }
 


Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -50,6 +50,9 @@
 # CHECK-NEXT:"storage.type.primitive.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
+# CHECK-NEXT:"meta.disabled"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.function.preprocessor.cpp"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -40,6 +40,7 @@
   Namespace,
   TemplateParameter,
   Primitive,
+

[PATCH] D67537: [WIP] [clangd] Client-side support for inactive regions

2019-09-29 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge updated this revision to Diff 222337.
nridge added a comment.
Herald added a subscriber: usaxena95.

Update to reflect changes to server side in D67536 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67537

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts


Index: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -190,6 +190,18 @@
 // highlighting becomes weird in certain cases.
 rangeBehavior : vscode.DecorationRangeBehavior.ClosedClosed,
   };
+  if (scopes[0] == "meta.disabled") {
+options.isWholeLine = true;
+// TODO: Avoid hardcoding these colors.
+options.light = {
+  color: "rgb(100, 100, 100)",
+  backgroundColor: "rgba(220, 220, 220, 0.3)"
+};
+options.dark = {
+  color: "rgb(100, 100, 100)",
+  backgroundColor: "rgba(18, 18, 18, 0.3)"
+}
+  }
   return vscode.window.createTextEditorDecorationType(options);
 });
 this.getVisibleTextEditorUris().forEach((fileUri) =>


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -190,6 +190,18 @@
 // highlighting becomes weird in certain cases.
 rangeBehavior : vscode.DecorationRangeBehavior.ClosedClosed,
   };
+  if (scopes[0] == "meta.disabled") {
+options.isWholeLine = true;
+// TODO: Avoid hardcoding these colors.
+options.light = {
+  color: "rgb(100, 100, 100)",
+  backgroundColor: "rgba(220, 220, 220, 0.3)"
+};
+options.dark = {
+  color: "rgb(100, 100, 100)",
+  backgroundColor: "rgba(18, 18, 18, 0.3)"
+}
+  }
   return vscode.window.createTextEditorDecorationType(options);
 });
 this.getVisibleTextEditorUris().forEach((fileUri) =>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

2019-09-29 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge added a comment.

The updated patch formulates this as a new semantic highlighting kind.

The tokens created for inactive regions are one per line, with the character 
offset and length being zero; the idea is that the client will handle this 
highlighting specially (see D67537 ) and 
highlight the entire line.

Note, highlighting the entire line is different than highlighting just the 
portion of the line that contains text. We want the grey background highlight 
to extend to the entire row in the editor, even over columns beyond the end of 
the text line (as accomplished with vscode's `isWholeLine=true` option in 
`DecorationRenderOptions`).

I don't love special-casing the scope in the client, but this does avoid having 
to extend the protocol itself which would be considerably more work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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

[PATCH] D66990: [clangd] Add distinct highlightings for declarations of functions and methods

2019-09-29 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge added a comment.
Herald added a subscriber: usaxena95.

In D66990#1656230 , @nridge wrote:

> >> I think the way cquery does it 
> >> 
> >>  it better in this regard: in place of a single kind enum, they 
> >> essentially have a 4-tuple of `(kind, parent kind, storage class, role)`.
> > 
> > A design like this definitely makes more sense. I was thinking of a 
> > slightly simpler model: adding a set of "modifiers" to each highlighting 
> > should be enough to encode all of it, e.g. a modifier 'is class member' 
> > could be used to distinguish methods and fields from global functions and 
> > variables, a modifier 'is usage' can be used to distinguish usages from 
> > declarations, etc.
> >  But there's no combinatorial explosion in the cquery's model, which is the 
> > important bit.
>
> I will suggest this for the upstream protocol and see where that goes.


For reference, I made this suggestion here 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66990



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

[PATCH] D68142: [Alignment][NFC] Remove LoadInst::setAlignment(unsigned)

2019-09-29 Thread Clement Courbet via Phabricator via cfe-commits 
courbet accepted this revision.
courbet added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:1273
   SomeLoad->getAAMetadata(AATags);
-  unsigned Align = SomeLoad->getAlignment();
+  MaybeAlign Align = MaybeAlign(SomeLoad->getAlignment());
 

const


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68142



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