[PATCH] D92024: [clang] Implement P0692R1 from C++20 (access checking on specializations and instantiations)

[Alex Orlov via Phabricator via cfe-commits]

aorlov updated this revision to Diff 311431.
aorlov added reviewers: mibintc, hokein.
aorlov added subscribers: mibintc, hokein.
aorlov added a comment.

Updated. Disabled function parameters access checking in function templates.
Hi, @broadwaylamb, @rsmith, @saar.raz, @doug.gregor, @mibintc, @hokein. Could 
you please look at this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92024

Files:
  clang/include/clang/Sema/DelayedDiagnostic.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/test/CXX/class.access/class.friend/p1.cpp
  clang/test/CXX/drs/dr1xx.cpp
  clang/test/CXX/temp/temp.spec/func.spec.cpp
  clang/test/CXX/temp/temp.spec/part.spec.cpp

Index: clang/test/CXX/temp/temp.spec/part.spec.cpp
===
--- /dev/null
+++ clang/test/CXX/temp/temp.spec/part.spec.cpp
@@ -0,0 +1,640 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// C++20 [temp.class.spec] 17.6.5/10:
+//   The usual access checking rules do not apply to non-dependent names used
+//   to specify template arguments of the simple-template-id of the partial
+//   specialization.
+
+// TODO: add test cases for `enum`
+
+// class for tests
+class TestClass {
+public:
+  class PublicClass {};
+  template  class TemplatePublicClass {};
+
+  using AliasPublicClass = unsigned char;
+
+  void publicFunc();
+  void publicFuncOverloaded();
+  void publicFuncOverloaded(int);
+
+  static void publicStaticFunc();
+  static void publicStaticFuncOverloaded();
+  static void publicStaticFuncOverloaded(int);
+
+  static constexpr int publicStaticInt = 42;
+
+protected:
+  // expected-note@+1 8{{declared protected here}}
+  class ProtectedClass {};
+  template  class TemplateProtectedClass {};
+
+  // expected-note@+1 2{{declared protected here}}
+  using AliasProtectedClass = const char;
+
+  // expected-note@+1 3{{declared protected here}}
+  void protectedFunc();
+  void protectedFuncOverloaded();
+  void protectedFuncOverloaded(int);
+
+  // expected-note@+1 2{{declared protected here}}
+  static void protectedStaticFunc();
+  // expected-note@+1 2{{declared protected here}}
+  static void protectedStaticFuncOverloaded();
+  static void protectedStaticFuncOverloaded(int);
+
+  // expected-note@+1 2{{declared protected here}}
+  static constexpr int protectedStaticInt = 43;
+
+private:
+  // expected-note@+1 10{{declared private here}}
+  class PrivateClass {};
+  // expected-note@+1 {{declared private here}}
+  template  class TemplatePrivateClass {};
+
+  using AliasPrivateClass = char *;
+
+  void privateFunc();
+  void privateFuncOverloaded();
+  void privateFuncOverloaded(int);
+
+  static void privateStaticFunc();
+  static void privateStaticFuncOverloaded();
+  static void privateStaticFuncOverloaded(int);
+
+  static constexpr int privateStaticInt = 44;
+};
+
+void globalFunction() {}
+
+//--//
+
+// template declarations for explicit instantiations
+template  class IT1 {};
+template  class IT2 {};
+template  class IT3 {};
+template  class IT4 {};
+template  class IT5 {};
+template  class IT6 {
+  template  class NIT1 {};
+};
+template  class IT7 {};
+template  class IT8 {};
+template  class IT9 {};
+
+// explicit instantiations
+
+// public
+template class IT1;
+template struct IT1>;
+template class IT1;
+template struct IT2;
+template class IT3;
+template struct IT4<&TestClass::publicFunc>;
+template class IT4<&TestClass::publicFuncOverloaded>;
+template class IT5<&TestClass::publicStaticFunc>;
+template class IT5<&TestClass::publicStaticFuncOverloaded>;
+template class IT5<&globalFunction>;
+template class IT6::template NIT1;
+template class IT7;
+template struct IT7>;
+template class IT8<&TestClass::publicFunc, TestClass::publicStaticInt>;
+template class IT8<&TestClass::publicFuncOverloaded, TestClass::publicStaticInt>;
+template class IT9;
+template class IT9;
+template class IT9;
+
+// protected
+template class IT1;
+template struct IT1>;
+template class IT1;
+template struct IT2;
+template class IT3;
+template struct IT4<&TestClass::protectedFunc>;
+template class IT4<&TestClass::protectedFuncOverloaded>;
+template class IT5<&TestClass::protectedStaticFunc>;
+template class IT5<&TestClass::protectedStaticFuncOverloaded>;
+template class IT6::template NIT1;
+template class IT7;
+template struct IT7>;
+template class IT8<&TestClass::protectedFunc, TestClass::protectedStaticInt>;
+template class IT8<&TestClass::protectedFuncOverloaded, TestClass::protectedStaticInt>;
+template class IT9;
+template class IT9;
+template class IT9;
+
+// private
+template class IT1;
+template struct IT1>;
+template class IT1;
+template struct IT2;
+template class IT3;
+template struct IT4<&TestClass::privateFunc>;
+template class IT4<&TestClass::privateFuncOverloaded>;
+template class IT5<&TestCl

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

(Also, for studying the effect of this one, on an IR level, the difference is 
already present in the output of `-S -emit-llvm` from clang, unless using 
`-Xclang -disable-llvm-passes`, but I'm sure you already took that into 
account.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87188

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

[clang] 05d1729 - [VE] Optimize toolchain regression test

[Kazushi Marukawa via cfe-commits]

Author: Kazushi (Jam) Marukawa
Date: 2020-12-13T20:26:05+09:00
New Revision: 05d1729232cdff323cafd469532504aa85740967

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

LOG: [VE] Optimize toolchain regression test

Optimize toolchain regression test for VE by removing not a useful test
(-fuse-init-array test) and merge several tests to one test which checks
default behavior of driver.  Also add sysroot to reduce conflicts.

These are suggested in https://reviews.llvm.org/D92996.
Thank you so much.

Reviewed By: MaskRay

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

Added: 
clang/test/Driver/Inputs/basic_ve_tree/opt/nec/ve/lib/crt1.o
clang/test/Driver/Inputs/basic_ve_tree/opt/nec/ve/lib/crti.o
clang/test/Driver/Inputs/basic_ve_tree/opt/nec/ve/lib/crtn.o

clang/test/Driver/Inputs/basic_ve_tree/resource_dir/lib/linux/clang_rt.crtbegin-ve.o

clang/test/Driver/Inputs/basic_ve_tree/resource_dir/lib/linux/clang_rt.crtend-ve.o

clang/test/Driver/Inputs/basic_ve_tree/resource_dir/lib/linux/libclang_rt.builtins-ve.a

Modified: 
clang/test/Driver/ve-toolchain.c
clang/test/Driver/ve-toolchain.cpp

Removed: 




diff  --git a/clang/test/Driver/Inputs/basic_ve_tree/opt/nec/ve/lib/crt1.o 
b/clang/test/Driver/Inputs/basic_ve_tree/opt/nec/ve/lib/crt1.o
new file mode 100644
index ..e69de29bb2d1

diff  --git a/clang/test/Driver/Inputs/basic_ve_tree/opt/nec/ve/lib/crti.o 
b/clang/test/Driver/Inputs/basic_ve_tree/opt/nec/ve/lib/crti.o
new file mode 100644
index ..e69de29bb2d1

diff  --git a/clang/test/Driver/Inputs/basic_ve_tree/opt/nec/ve/lib/crtn.o 
b/clang/test/Driver/Inputs/basic_ve_tree/opt/nec/ve/lib/crtn.o
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/Driver/Inputs/basic_ve_tree/resource_dir/lib/linux/clang_rt.crtbegin-ve.o
 
b/clang/test/Driver/Inputs/basic_ve_tree/resource_dir/lib/linux/clang_rt.crtbegin-ve.o
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/Driver/Inputs/basic_ve_tree/resource_dir/lib/linux/clang_rt.crtend-ve.o
 
b/clang/test/Driver/Inputs/basic_ve_tree/resource_dir/lib/linux/clang_rt.crtend-ve.o
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/Driver/Inputs/basic_ve_tree/resource_dir/lib/linux/libclang_rt.builtins-ve.a
 
b/clang/test/Driver/Inputs/basic_ve_tree/resource_dir/lib/linux/libclang_rt.builtins-ve.a
new file mode 100644
index ..e69de29bb2d1

diff  --git a/clang/test/Driver/ve-toolchain.c 
b/clang/test/Driver/ve-toolchain.c
index 0ca3c84373f3..ac925e470770 100644
--- a/clang/test/Driver/ve-toolchain.c
+++ b/clang/test/Driver/ve-toolchain.c
@@ -7,83 +7,93 @@
 // RUN: %clang -### -g -target ve %s 2>&1 | FileCheck -check-prefix=DWARF_VER 
%s
 // DWARF_VER: "-dwarf-version=4"
 
-///-
-/// Checking dynamic-linker
-
-// RUN: %clang -### -target ve %s 2>&1 | FileCheck -check-prefix=DYNLINKER %s
-// DYNLINKER: nld{{.*}} "-dynamic-linker" "/opt/nec/ve/lib/ld-linux-ve.so.1"
-
-///-
-/// Checking VE specific option
-
-// RUN: %clang -### -target ve %s 2>&1 | FileCheck -check-prefix=VENLDOPT %s
-// VENLDOPT: nld{{.*}} "-z" "max-page-size=0x400"
-
 
///-
 /// Checking include-path
 
-// RUN: %clang -### -target ve %s 2>&1 | FileCheck -check-prefix=DEFINC %s
+// RUN: %clang -### -target ve --sysroot %S/Inputs/basic_ve_tree %s \
+// RUN: -resource-dir=%S/Input/basic_ve_tree/resource_dir \
+// RUN: 2>&1 | FileCheck -check-prefix=DEFINC %s
 // DEFINC: clang{{.*}} "-cc1"
-// DEFINC: "-nostdsysteminc"
-// DEFINC: "-internal-isystem" "{{.*}}/lib/clang/{{[0-9.]*}}/include"
-// DEFINC: "-internal-isystem" "/opt/nec/ve/include"
-
-// RUN: %clang -### -target ve %s -nostdlibinc 2>&1 | \
-// RUN:FileCheck -check-prefix=NOSTDLIBINC %s
+// DEFINC-SAME: "-nostdsysteminc"
+// DEFINC-SAME: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// DEFINC-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
+// DEFINC-SAME: "-internal-isystem" "[[RESOURCE_DIR]]/include"
+// DEFINC-SAME: "-internal-isystem" "[[SYSROOT]]/opt/nec/ve/include"
+
+// RUN: %clang -### -target ve --sysroot %S/Inputs/basic_ve_tree %s \
+// RUN: -resource-dir=%S/Input/basic_ve_tree/resource_dir \
+// RUN: -nostdlibinc 2>&1 | FileCheck -check-prefix=NOSTDLIBINC %s
 // NOSTDLIBINC: clang{{.*}} "-cc1"
-// NOSTDLIBINC: "-internal-isystem" "{{.*}}/lib/clang/{{[0-9.]*}}/include"
-// NOSTDLIBINC-NOT: "-internal-isystem" "/opt/nec/ve/include"
-
-// RUN: %clang -### -target ve %s -nobuiltininc 2>&1 | \
-// RUN:FileCheck -check-prefix=NOBUILT

[PATCH] D93084: [VE] Optimize toolchain regression test

[Kazushi Marukawa via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG05d1729232cd: [VE] Optimize toolchain regression test 
(authored by kaz7).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93084

Files:
  clang/test/Driver/Inputs/basic_ve_tree/opt/nec/ve/lib/crt1.o
  clang/test/Driver/Inputs/basic_ve_tree/opt/nec/ve/lib/crti.o
  clang/test/Driver/Inputs/basic_ve_tree/opt/nec/ve/lib/crtn.o
  
clang/test/Driver/Inputs/basic_ve_tree/resource_dir/lib/linux/clang_rt.crtbegin-ve.o
  
clang/test/Driver/Inputs/basic_ve_tree/resource_dir/lib/linux/clang_rt.crtend-ve.o
  
clang/test/Driver/Inputs/basic_ve_tree/resource_dir/lib/linux/libclang_rt.builtins-ve.a
  clang/test/Driver/ve-toolchain.c
  clang/test/Driver/ve-toolchain.cpp

Index: clang/test/Driver/ve-toolchain.cpp
===
--- clang/test/Driver/ve-toolchain.cpp
+++ clang/test/Driver/ve-toolchain.cpp
@@ -7,111 +7,125 @@
 // RUN: %clangxx -### -g -target ve %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s
 // DWARF_VER: "-dwarf-version=4"
 
-///-
-/// Checking VE specific option
-
-// RUN: %clangxx -### -target ve %s 2>&1 | FileCheck -check-prefix=VENLDOPT %s
-// VENLDOPT: nld{{.*}} "-z" "max-page-size=0x400"
-
 ///-
 /// Checking include-path
 
-// RUN: %clangxx -### -target ve %s 2>&1 | FileCheck -check-prefix=DEFINC %s
+// RUN: %clangxx -### -target ve --sysroot %S/Inputs/basic_ve_tree %s \
+// RUN: -resource-dir=%S/Input/basic_ve_tree/resource_dir \
+// RUN: 2>&1 | FileCheck -check-prefix=DEFINC %s
 // DEFINC: clang{{.*}} "-cc1"
-// DEFINC: "-nostdsysteminc"
-// DEFINC: "-internal-isystem" "{{.*}}/lib/clang/{{[0-9.]*}}/include/c++/v1"
-// DEFINC: "-internal-isystem" "{{.*}}/lib/clang/{{[0-9.]*}}/include"
-// DEFINC: "-internal-isystem" "/opt/nec/ve/include"
-
-// RUN: %clangxx -### -target ve %s -nostdlibinc 2>&1 | \
-// RUN:FileCheck -check-prefix=NOSTDLIBINC %s
+// DEFINC-SAME: "-nostdsysteminc"
+// DEFINC-SAME: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// DEFINC-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
+// DEFINC-SAME: "-internal-isystem" "[[RESOURCE_DIR]]/include/c++/v1"
+// DEFINC-SAME: "-internal-isystem" "[[RESOURCE_DIR]]/include"
+// DEFINC-SAME: "-internal-isystem" "[[SYSROOT]]/opt/nec/ve/include"
+
+// RUN: %clangxx -### -target ve --sysroot %S/Inputs/basic_ve_tree %s \
+// RUN: -resource-dir=%S/Input/basic_ve_tree/resource_dir \
+// RUN: -nostdlibinc 2>&1 | FileCheck -check-prefix=NOSTDLIBINC %s
 // NOSTDLIBINC: clang{{.*}} "-cc1"
-// NOSTDLIBINC-NOT: "-internal-isystem" "{{.*}}/lib/clang/{{[0-9.]*}}/include/c++/v1"
-// NOSTDLIBINC: "-internal-isystem" "{{.*}}/lib/clang/{{[0-9.]*}}/include"
-// NOSTDLIBINC-NOT: "-internal-isystem" "/opt/nec/ve/include"
-
-// RUN: %clangxx -### -target ve %s -nobuiltininc 2>&1 | \
-// RUN:FileCheck -check-prefix=NOBUILTININC %s
+// NOSTDLIBINC-SAME: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// NOSTDLIBINC-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
+// NOSTDLIBINC-NOT: "-internal-isystem" "[[RESOURCE_DIR]]/include/c++/v1"
+// NOSTDLIBINC-SAME: "-internal-isystem" "[[RESOURCE_DIR]]/include"
+// NOSTDLIBINC-NOT: "-internal-isystem" "[[SYSROOT]]/opt/nec/ve/include"
+
+// RUN: %clangxx -### -target ve --sysroot %S/Inputs/basic_ve_tree %s \
+// RUN: -resource-dir=%S/Input/basic_ve_tree/resource_dir \
+// RUN: -nobuiltininc 2>&1 | FileCheck -check-prefix=NOBUILTININC %s
 // NOBUILTININC: clang{{.*}} "-cc1"
-// NOBUILTININC: "-nobuiltininc"
-// NOBUILTININC: "-internal-isystem" "{{.*}}/lib/clang/{{[0-9.]*}}/include/c++/v1"
-// NOBUILTININC-NOT: "-internal-isystem" "{{.*}}/lib/clang/{{[0-9.]*}}/include"
-// NOBUILTININC: "-internal-isystem" "/opt/nec/ve/include"
-
-// RUN: %clangxx -### -target ve %s -nostdinc 2>&1 | \
-// RUN:FileCheck -check-prefix=NOSTDINC %s
+// NOBUILTININC-SAME: "-nobuiltininc"
+// NOBUILTININC-SAME: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// NOBUILTININC-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
+// NOBUILTININC-SAME: "-internal-isystem" "[[RESOURCE_DIR]]/include/c++/v1"
+// NOBUILTININC-NOT: "-internal-isystem" "[[RESOURCE_DIR]]/include"
+// NOBUILTININC-SAME: "-internal-isystem" "[[SYSROOT]]/opt/nec/ve/include"
+
+// RUN: %clangxx -### -target ve --sysroot %S/Inputs/basic_ve_tree %s \
+// RUN: -resource-dir=%S/Input/basic_ve_tree/resource_dir \
+// RUN: -nostdinc 2>&1 | FileCheck -check-prefix=NOSTDINC %s
 // NOSTDINC: clang{{.*}} "-cc1"
-// NOSTDINC: "-nobuiltininc"
-// NOSTDINC-NOT: "-internal-isystem" "{{.*}}/lib/clang/{{[0-9.]*}}/include/c++/v1"
-// NOSTDINC-NOT: "-internal-isystem" "{{.*}}/lib/clang/{{[0-9.]*}}/include"
-// NOSTDINC-NOT: "-internal-isystem" "/opt/nec/ve/include"
-
-// R

[PATCH] D93084: [VE] Optimize toolchain regression test

[Kazushi Marukawa via Phabricator via cfe-commits]

kaz7 added a comment.

Thank you so much too.  I could learn how to write better clang tests.  :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93084

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

[PATCH] D93170: [clang-format][NFC] Expand BreakBeforeBraces examples

[MyDeveloperDay via Phabricator via cfe-commits]

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

We should have the same examples added to the unit tests so we know they are 
correct (and stay correct)

But I really appreciate the documentation effort


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93170

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

[clang] 3630640 - [clang-format] Remove double trim

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2020-12-13T14:16:54+01:00
New Revision: 36306403d492d4a4b54c72c6c4c511021584243b

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

LOG: [clang-format] Remove double trim

Lines[i] is already trimmed 3 lines before

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

Added: 


Modified: 
clang/lib/Format/BreakableToken.cpp

Removed: 




diff  --git a/clang/lib/Format/BreakableToken.cpp 
b/clang/lib/Format/BreakableToken.cpp
index 4975c89164a4..ea5cc31af07a 100644
--- a/clang/lib/Format/BreakableToken.cpp
+++ b/clang/lib/Format/BreakableToken.cpp
@@ -773,10 +773,7 @@ BreakableLineCommentSection::BreakableLineCommentSection(
 OriginalPrefix.resize(Lines.size());
 for (size_t i = FirstLineIndex, e = Lines.size(); i < e; ++i) {
   Lines[i] = Lines[i].ltrim(Blanks);
-  // We need to trim the blanks in case this is not the first line in a
-  // multiline comment. Then the indent is included in Lines[i].
-  StringRef IndentPrefix =
-  getLineCommentIndentPrefix(Lines[i].ltrim(Blanks), Style);
+  StringRef IndentPrefix = getLineCommentIndentPrefix(Lines[i], Style);
   assert((TokenText.startswith("//") || TokenText.startswith("#")) &&
  "unsupported line comment prefix, '//' and '#' are supported");
   OriginalPrefix[i] = Prefix[i] = IndentPrefix;



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

[PATCH] D91996: [clang-format] Remove double trim

[Björn Schäpers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG36306403d492: [clang-format] Remove double trim (authored by 
HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91996

Files:
  clang/lib/Format/BreakableToken.cpp


Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -773,10 +773,7 @@
 OriginalPrefix.resize(Lines.size());
 for (size_t i = FirstLineIndex, e = Lines.size(); i < e; ++i) {
   Lines[i] = Lines[i].ltrim(Blanks);
-  // We need to trim the blanks in case this is not the first line in a
-  // multiline comment. Then the indent is included in Lines[i].
-  StringRef IndentPrefix =
-  getLineCommentIndentPrefix(Lines[i].ltrim(Blanks), Style);
+  StringRef IndentPrefix = getLineCommentIndentPrefix(Lines[i], Style);
   assert((TokenText.startswith("//") || TokenText.startswith("#")) &&
  "unsupported line comment prefix, '//' and '#' are supported");
   OriginalPrefix[i] = Prefix[i] = IndentPrefix;


Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -773,10 +773,7 @@
 OriginalPrefix.resize(Lines.size());
 for (size_t i = FirstLineIndex, e = Lines.size(); i < e; ++i) {
   Lines[i] = Lines[i].ltrim(Blanks);
-  // We need to trim the blanks in case this is not the first line in a
-  // multiline comment. Then the indent is included in Lines[i].
-  StringRef IndentPrefix =
-  getLineCommentIndentPrefix(Lines[i].ltrim(Blanks), Style);
+  StringRef IndentPrefix = getLineCommentIndentPrefix(Lines[i], Style);
   assert((TokenText.startswith("//") || TokenText.startswith("#")) &&
  "unsupported line comment prefix, '//' and '#' are supported");
   OriginalPrefix[i] = Prefix[i] = IndentPrefix;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92277: [OpenCL] Refactor of targets OpenCL option settings

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added inline comments.



Comment at: clang/include/clang/Basic/OpenCLOptions.h:22
 
-/// OpenCL supported extensions and optional core features
-class OpenCLOptions {
-  struct Info {
-bool Supported; // Is this option supported
-bool Enabled;   // Is this option enabled
-unsigned Avail; // Option starts to be available in this OpenCL version
-unsigned Core;  // Option becomes (optional) core feature in this OpenCL
-// version
-Info(bool S = false, bool E = false, unsigned A = 100, unsigned C = ~0U)
-  :Supported(S), Enabled(E), Avail(A), Core(C){}
-  };
-  llvm::StringMap OptMap;
-public:
-  /// Check if \c Ext is a recognized OpenCL extension.
-  ///
-  /// \param Ext - Extension to look up.
-  /// \returns \c true if \c Ext is known, \c false otherwise.
-  bool isKnown(llvm::StringRef Ext) const {
-return OptMap.find(Ext) != OptMap.end();
-  }
+enum OpenCLVersionID : unsigned int {
+  OCL_C_10 = 0x1,

Anastasia wrote:
> Let's add a comment to document this new enum.
I'm going to change comments all over the place in the next patch. Thanks!



Comment at: clang/include/clang/Basic/OpenCLOptions.h:36
+// OpenCL C version mask
+class OpenCLVersionEncoderHelper {
+private:

Anastasia wrote:
> Looking at the uses, it might be better to just have a helper function that 
> takes `OpenCLVersion` and `LangOpts` instead of introducing a class that is 
> only used as temporary.
Ok, that is reasonable. Will change.



Comment at: clang/include/clang/Basic/TargetInfo.h:1442
+  virtual void supportAllOpenCLOpts(bool V = true) {
+#define OPENCLEXTNAME(Ext) getTargetOpts().OpenCLFeaturesMap[#Ext] = V;
+#include "clang/Basic/OpenCLExtensions.def"

Anastasia wrote:
> Btw here you could just iterate over the elements in the map? This could be 
> faster than multiple independent look ups into the map. But mainly I think it 
> makes the flow a bit cleaneras it indicated that you only set elements and 
> not add them...
> Btw here you could just iterate over the elements in the map

Map is empty at this point. `::supportAllOpenCLOpts` adds all elements which 
are declared in `OpenCLExtensions.def` into the map.



Comment at: clang/lib/Basic/OpenCLOptions.cpp:116
+if (F.getValue().IsCoreIn(Opts))
+  support(F.getKey());
+}

Anastasia wrote:
> Btw what if the target doesn't support the feature? Do you think we could 
> provide an error?
I think that if a feature is //core// than a target //must// support it, do I 
miss something?

However, there is a place for such diagnostics for non-core features already: 
something similar to `TargetInfo::handleTargetFeatures(std::vector 
&Features, DiagnosticsEngine &Diags)` could be used



Comment at: clang/lib/Basic/Targets.cpp:725
+if ((It != OpenCLFeaturesMap.end() && It->getValue()) ||
+OpenCLOptionInfo(AvailVer, CoreVersions, OptionalVersions)
+.IsCoreIn(Opts))

Anastasia wrote:
> I think we should find different place for those now.
> 
> Ideally, we should iterate though the map in `OpenCLOptions` and set the 
> define for the supported ones.
> 
> I suggest we move this into `clang::InitializePreprocessor` which means 
> `Preprocessor` might need a reference to `OpenCLOptions` which is the right 
> thing to do because it needs to know the features that require the macro, the 
> same as for `LangOpts`. This means we will need to change the initialization 
> time of `OpenCLOptions` member in Sema or potentially even reparent it 
> elsewhere... perhaps to `CompilerInstance` where `LangOpts` and `TargetInfo` 
> seems to be created already?
> I suggest we move this into clang::InitializePreprocessor which means

I think I'm going to introduce `InitializeOpenCLFeatureTestMacros(TargetInfo, 
LangOptions)` in `InitPreprocessor.cpp`.

> This means we will need to change the initialization time of OpenCLOptions 
> member in Sema or potentially even reparent it elsewhere... perhaps to 
> CompilerInstance where LangOpts and TargetInfo seems to be created already

This seems too invasive for me since `CompilerInstance` is a different purpose 
class; and it already holds `Sema` and `TargetInfo`. `OpenCLOptions` should 
mainly be used for parsing, so I would like to suggest avoiding using it 
elsewhere. Furthermore, with the proposed flow `OpenCLOptions.h` can be removed 
later  and some simple helpers can be used to check the availability of 
features/extensions.

However, I see your point: we have two identical pieces of code in 
`TargetInfo::getOpenCLFeatureDefines` and `OpenCLOptions::addSupport`. I think 
I'll try to get rid of this duplication by transferring setting of core 
features into `TargetInfo::adjust` which seems pretty right thing to do. What 
do you think?



Comment at: clang/test/Misc/r600.languageOptsOpenCL.cl:26

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D92257#2438928 , @MyDeveloperDay 
wrote:

> Could we consider dropping the maximum?

While rewriting the tests with the unmodified clang-format I just confirmed 
that currently only the minimum of 1 is enforced, there is no maximum. I.e.

  //x
  int x;
  //   y
  int y;

will be formatted as

  // x
  int x;
  //   y
  int y;

So for what I want to do I can:

1. Enforce the Minimum (for LLVM Style 1)
2. Enforce an optional Maximum (but what if the Maximum is set to 0, what I 
want to do, given that the Minimum is 1?)
3. Stay with the current design of a Minimum/Maximum Pair with (practically 
unbounded Maximum)

I would choose #3, in that case I only have to add a test for line comment 
sections (and most likely adapt the implementation).

  /// A List:
  ///  * Foo
  ///  * Bar


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[PATCH] D92277: [OpenCL] Refactor of targets OpenCL option settings

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/test/Misc/r600.languageOptsOpenCL.cl:26
 // RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple r600-unknown-unknown 
-Wpedantic-core-features -DTEST_CORE_FEATURES -target-cpu turks
+// XFAIL: *
 

azabaznov wrote:
> yaxunl wrote:
> > Pls remove XFAIL
> Sure. I'll guard this #ifndef check with OpenCL version in existing tests; 
> but however, I think adding new tests with XFAIL for r600 and NVPTX (where 
> #ifndef check fails) seems reasonable to me. What do you think?
I think it may be reasonable to remove the run line for CL2.0 and corresponding 
checks for 2.0 specific core features/extensions from this lit test since r600 
does not support CL2.0.


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

https://reviews.llvm.org/D92277

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

[PATCH] D92940: [X86] Convert fadd/fmul _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: clang/lib/Headers/avx512fintrin.h:9559
 static __inline__ double __DEFAULT_FN_ATTRS512
 _mm512_reduce_max_pd(__m512d __V) {
   _mm512_mask_reduce_operator(max_pd);

pengfei wrote:
> Better to change min and max as well.
OK - I'll do that as a followup immediately after this patch - thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92940

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

[PATCH] D92940: [X86] Convert fadd/fmul _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

[Simon Pilgrim via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4855a1004d4d: [X86] Convert fadd/fmul _mm_reduce_* 
intrinsics to emit llvm.reduction… (authored by RKSimon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92940

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/avx512fintrin.h
  clang/test/CodeGen/X86/avx512-reduceIntrin.c

Index: clang/test/CodeGen/X86/avx512-reduceIntrin.c
===
--- clang/test/CodeGen/X86/avx512-reduceIntrin.c
+++ clang/test/CodeGen/X86/avx512-reduceIntrin.c
@@ -115,67 +115,25 @@
 
 double test_mm512_reduce_add_pd(__m512d __W){
 // CHECK-LABEL: @test_mm512_reduce_add_pd(
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:fadd <4 x double> %{{.*}}, %{{.*}}
-// CHECK:shufflevector <4 x double> %{{.*}}, <4 x double> undef, <2 x i32> 
-// CHECK:shufflevector <4 x double> %{{.*}}, <4 x double> undef, <2 x i32> 
-// CHECK:fadd <2 x double> %{{.*}}, %{{.*}}
-// CHECK:shufflevector <2 x double> %{{.*}}, <2 x double> %{{.*}}, <2 x i32> 
-// CHECK:fadd <2 x double> %{{.*}}, %{{.*}}
-// CHECK:extractelement <2 x double> %{{.*}}, i32 0
+// CHECK:call double @llvm.vector.reduce.fadd.v8f64(double 0.00e+00, <8 x double> %{{.*}})
   return _mm512_reduce_add_pd(__W); 
 }
 
 double test_mm512_reduce_mul_pd(__m512d __W){
 // CHECK-LABEL: @test_mm512_reduce_mul_pd(
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:fmul <4 x double> %{{.*}}, %{{.*}}
-// CHECK:shufflevector <4 x double> %{{.*}}, <4 x double> undef, <2 x i32> 
-// CHECK:shufflevector <4 x double> %{{.*}}, <4 x double> undef, <2 x i32> 
-// CHECK:fmul <2 x double> %{{.*}}, %{{.*}}
-// CHECK:shufflevector <2 x double> %{{.*}}, <2 x double> %{{.*}}, <2 x i32> 
-// CHECK:fmul <2 x double> %{{.*}}, %{{.*}}
-// CHECK:extractelement <2 x double> %{{.*}}, i32 0
+// CHECK:call double @llvm.vector.reduce.fmul.v8f64(double 1.00e+00, <8 x double> %{{.*}})
   return _mm512_reduce_mul_pd(__W); 
 }
 
 float test_mm512_reduce_add_ps(__m512 __W){
 // CHECK-LABEL: @test_mm512_reduce_add_ps(
-// CHECK:bitcast <16 x float> %{{.*}} to <8 x double>
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:bitcast <4 x double> %{{.*}} to <8 x float>
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:bitcast <4 x double> %{{.*}} to <8 x float>
-// CHECK:fadd <8 x float> %{{.*}}, %{{.*}}
-// CHECK:shufflevector <8 x float> %{{.*}}, <8 x float> undef, <4 x i32> 
-// CHECK:shufflevector <8 x float> %{{.*}}, <8 x float> undef, <4 x i32> 
-// CHECK:fadd <4 x float> %{{.*}}, %{{.*}}
-// CHECK:shufflevector <4 x float> %{{.*}}, <4 x float> %{{.*}}, <4 x i32> 
-// CHECK:fadd <4 x float> %{{.*}}, %{{.*}}
-// CHECK:shufflevector <4 x float> %{{.*}}, <4 x float> %{{.*}}, <4 x i32> 
-// CHECK:fadd <4 x float> %{{.*}}, %{{.*}}
-// CHECK:extractelement <4 x float> %{{.*}}, i32 0
+// CHECK:call float @llvm.vector.reduce.fadd.v16f32(float 0.00e+00, <16 x float> %{{.*}})
   return _mm512_reduce_add_ps(__W); 
 }
 
 float test_mm512_reduce_mul_ps(__m512 __W){
 // CHECK-LABEL: @test_mm512_reduce_mul_ps(
-// CHECK:bitcast <16 x float> %{{.*}} to <8 x double>
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:bitcast <4 x double> %{{.*}} to <8 x float>
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:bitcast <4 x double> %{{.*}} to <8 x float>
-// CHECK:fmul <8 x float> %{{.*}}, %{{.*}}
-// CHECK:shufflevector <8 x float> %{{.*}}, <8 x float> undef, <4 x i32> 
-// CHECK:shufflevector <8 x float> %{{.*}}, <8 x float> undef, <4 x i32> 
-// CHECK:fmul <4 x float> %{{.*}}, %{{.*}}
-// CHECK:shufflevector <4 x float> %{{.*}}, <4 x float> %{{.*}}, <4 x i32> 
-// CHECK:fmul <4 x float> %{{.*}}, %{{.*}}
-// CHECK:shufflevector <4 x float> %{{.*}}, <4 x float> %{{.*}}, <4 x i32> 
-// CHECK:fmul <4 x float> %{{.*}}, %{{.*}}
-// CHECK:extractelement <4 x float> %{{.*}}, i32 0
+// CHECK:call float @llvm.vector.reduce.fmul.v16f32(float 1.00e+00, <16 x float> %{{.*}})
   return _mm512_reduce_mul_ps(__W); 
 }
 
@@ -183,15 +141,7 @@
 // CHECK-LABEL: @test_mm512_mask_reduce_add_pd(
 // CHECK:bitcast i8 %{{.*}} to <8 x i1>
 // CHECK:select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}}
-// CHECK:shufflevector <8 

[clang] 4855a10 - [X86] Convert fadd/fmul _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-12-13T15:37:35Z
New Revision: 4855a1004d4d87b6c21c510c1724e74a8d37d91a

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

LOG: [X86] Convert fadd/fmul _mm_reduce_* intrinsics to emit llvm.reduction 
intrinsics (PR47506)

Followup to D87604, having confirmed on PR47506 that we can use the llvm 
codegen expansion for fadd/fmul as well.

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsX86.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Headers/avx512fintrin.h
clang/test/CodeGen/X86/avx512-reduceIntrin.c

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsX86.def 
b/clang/include/clang/Basic/BuiltinsX86.def
index 0f5594f1a4e6..16fb7dd7b0e6 100644
--- a/clang/include/clang/Basic/BuiltinsX86.def
+++ b/clang/include/clang/Basic/BuiltinsX86.def
@@ -1876,6 +1876,10 @@ TARGET_BUILTIN(__builtin_ia32_reduce_add_d512, "iV16i", 
"ncV:512:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_reduce_add_q512, "OiV8Oi", "ncV:512:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_reduce_and_d512, "iV16i", "ncV:512:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_reduce_and_q512, "OiV8Oi", "ncV:512:", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_fadd_pd512, "ddV8d", "ncV:512:", 
"avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_fadd_ps512, "ffV16f", "ncV:512:", 
"avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_fmul_pd512, "ddV8d", "ncV:512:", 
"avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_fmul_ps512, "ffV16f", "ncV:512:", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_reduce_mul_d512, "iV16i", "ncV:512:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_reduce_mul_q512, "OiV8Oi", "ncV:512:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_reduce_or_d512, "iV16i", "ncV:512:", "avx512f")

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 316a60c31fd4..74f6c9fee2c8 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -13631,6 +13631,18 @@ Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned 
BuiltinID,
 CGM.getIntrinsic(Intrinsic::vector_reduce_and, Ops[0]->getType());
 return Builder.CreateCall(F, {Ops[0]});
   }
+  case X86::BI__builtin_ia32_reduce_fadd_pd512:
+  case X86::BI__builtin_ia32_reduce_fadd_ps512: {
+Function *F =
+CGM.getIntrinsic(Intrinsic::vector_reduce_fadd, Ops[1]->getType());
+return Builder.CreateCall(F, {Ops[0], Ops[1]});
+  }
+  case X86::BI__builtin_ia32_reduce_fmul_pd512:
+  case X86::BI__builtin_ia32_reduce_fmul_ps512: {
+Function *F =
+CGM.getIntrinsic(Intrinsic::vector_reduce_fmul, Ops[1]->getType());
+return Builder.CreateCall(F, {Ops[0], Ops[1]});
+  }
   case X86::BI__builtin_ia32_reduce_mul_d512:
   case X86::BI__builtin_ia32_reduce_mul_q512: {
 Function *F =

diff  --git a/clang/lib/Headers/avx512fintrin.h 
b/clang/lib/Headers/avx512fintrin.h
index 2df399d978e3..2ee4350b14d4 100644
--- a/clang/lib/Headers/avx512fintrin.h
+++ b/clang/lib/Headers/avx512fintrin.h
@@ -9345,37 +9345,25 @@ _mm512_mask_reduce_or_epi64(__mmask8 __M, __m512i __W) {
   return __builtin_ia32_reduce_or_q512(__W);
 }
 
-#define _mm512_mask_reduce_operator(op) \
-  __m256d __t1 = _mm512_extractf64x4_pd(__W, 0); \
-  __m256d __t2 = _mm512_extractf64x4_pd(__W, 1); \
-  __m256d __t3 = __t1 op __t2; \
-  __m128d __t4 = _mm256_extractf128_pd(__t3, 0); \
-  __m128d __t5 = _mm256_extractf128_pd(__t3, 1); \
-  __m128d __t6 = __t4 op __t5; \
-  __m128d __t7 = __builtin_shufflevector(__t6, __t6, 1, 0); \
-  __m128d __t8 = __t6 op __t7; \
-  return __t8[0]
-
 static __inline__ double __DEFAULT_FN_ATTRS512 _mm512_reduce_add_pd(__m512d 
__W) {
-  _mm512_mask_reduce_operator(+);
+  return __builtin_ia32_reduce_fadd_pd512(0.0, __W);
 }
 
 static __inline__ double __DEFAULT_FN_ATTRS512 _mm512_reduce_mul_pd(__m512d 
__W) {
-  _mm512_mask_reduce_operator(*);
+  return __builtin_ia32_reduce_fmul_pd512(1.0, __W);
 }
 
 static __inline__ double __DEFAULT_FN_ATTRS512
 _mm512_mask_reduce_add_pd(__mmask8 __M, __m512d __W) {
   __W = _mm512_maskz_mov_pd(__M, __W);
-  _mm512_mask_reduce_operator(+);
+  return __builtin_ia32_reduce_fadd_pd512(0.0, __W);
 }
 
 static __inline__ double __DEFAULT_FN_ATTRS512
 _mm512_mask_reduce_mul_pd(__mmask8 __M, __m512d __W) {
   __W = _mm512_mask_mov_pd(_mm512_set1_pd(1.0), __M, __W);
-  _mm512_mask_reduce_operator(*);
+  return __builtin_ia32_reduce_fmul_pd512(1.0, __W);
 }
-#undef _mm512_mask_reduce_operator
 
 static __inline__ int __DEFAULT_FN_ATTRS512
 _mm512_reduce_add_epi32(__m512i __W) {
@@ -9421,41 +9409,27 @@ _mm512_mask_reduce_or_epi32(__mmask16 __M, __m512i __W) 
{
   return __builtin_ia32_reduce_or_d512((__v16si)__W);
 }
 
-#define _mm512_mask_reduce_operator(op) \
-  __m256 __t1

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon created this revision.
RKSimon added reviewers: craig.topper, pengfei, spatel.
RKSimon requested review of this revision.
Herald added a project: clang.

As suggested by @pengfei on D92940 

My concern with this is that by default llvm.vector.reduce.fmax/min matches the 
fmaxnum/fminnum behaviour for nan values, which will scalarize on x86. But the 
existing clang/gcc/icc behaviour is to effectively to treat this as a fastmath 
vector reduction using the AVX vmaxpd/vminpd ops.

The intel descriptions for these _mm_reduce_* intrinsics is very vague - are we 
happy to always set these expansions to fast math? Or is this use of the llvm 
intrinsics a step too far?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93179

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/avx512fintrin.h
  clang/test/CodeGen/X86/avx512-reduceMinMaxIntrin.c

Index: clang/test/CodeGen/X86/avx512-reduceMinMaxIntrin.c
===
--- clang/test/CodeGen/X86/avx512-reduceMinMaxIntrin.c
+++ clang/test/CodeGen/X86/avx512-reduceMinMaxIntrin.c
@@ -16,15 +16,7 @@
 
 double test_mm512_reduce_max_pd(__m512d __W){
 // CHECK-LABEL: @test_mm512_reduce_max_pd(
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:call <4 x double> @llvm.x86.avx.max.pd.256(<4 x double> %{{.*}}, <4 x double> %{{.*}})
-// CHECK:shufflevector <4 x double> %{{.*}}, <4 x double> undef, <2 x i32> 
-// CHECK:shufflevector <4 x double> %{{.*}}, <4 x double> undef, <2 x i32> 
-// CHECK:call <2 x double> @llvm.x86.sse2.max.pd(<2 x double> %{{.*}}, <2 x double> %{{.*}})
-// CHECK:shufflevector <2 x double> %{{.*}}, <2 x double> %{{.*}}, <2 x i32> 
-// CHECK:call <2 x double> @llvm.x86.sse2.max.pd(<2 x double> %{{.*}}, <2 x double> %{{.*}})
-// CHECK:extractelement <2 x double> %{{.*}}, i32 0
+// CHECK:call double @llvm.vector.reduce.fmax.v8f64(<8 x double> %{{.*}})
   return _mm512_reduce_max_pd(__W); 
 }
 
@@ -42,15 +34,7 @@
 
 double test_mm512_reduce_min_pd(__m512d __W){
 // CHECK-LABEL: @test_mm512_reduce_min_pd(
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:call <4 x double> @llvm.x86.avx.min.pd.256(<4 x double> %{{.*}}, <4 x double> %{{.*}})
-// CHECK:shufflevector <4 x double> %{{.*}}, <4 x double> undef, <2 x i32> 
-// CHECK:shufflevector <4 x double> %{{.*}}, <4 x double> undef, <2 x i32> 
-// CHECK:call <2 x double> @llvm.x86.sse2.min.pd(<2 x double> %{{.*}}, <2 x double> %{{.*}})
-// CHECK:shufflevector <2 x double> %{{.*}}, <2 x double> %{{.*}}, <2 x i32> 
-// CHECK:call <2 x double> @llvm.x86.sse2.min.pd(<2 x double> %{{.*}}, <2 x double> %{{.*}})
-// CHECK:extractelement <2 x double> %{{.*}}, i32 0
+// CHECK:call double @llvm.vector.reduce.fmin.v8f64(<8 x double> %{{.*}})
   return _mm512_reduce_min_pd(__W); 
 }
 
@@ -74,15 +58,7 @@
 // CHECK-LABEL: @test_mm512_mask_reduce_max_pd(
 // CHECK:bitcast i8 %{{.*}} to <8 x i1>
 // CHECK:select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}}
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:call <4 x double> @llvm.x86.avx.max.pd.256(<4 x double> %{{.*}}, <4 x double> %{{.*}})
-// CHECK:shufflevector <4 x double> %{{.*}}, <4 x double> undef, <2 x i32> 
-// CHECK:shufflevector <4 x double> %{{.*}}, <4 x double> undef, <2 x i32> 
-// CHECK:call <2 x double> @llvm.x86.sse2.max.pd(<2 x double> %{{.*}}, <2 x double> %{{.*}})
-// CHECK:shufflevector <2 x double> %{{.*}}, <2 x double>  %{{.*}}, <2 x i32> 
-// CHECK:call <2 x double> @llvm.x86.sse2.max.pd(<2 x double> %{{.*}}, <2 x double> %{{.*}})
-// CHECK:extractelement <2 x double> %{{.*}}, i32 0
+// CHECK:call double @llvm.vector.reduce.fmax.v8f64(<8 x double> %{{.*}})
   return _mm512_mask_reduce_max_pd(__M, __W); 
 }
 
@@ -106,15 +82,7 @@
 // CHECK-LABEL: @test_mm512_mask_reduce_min_pd(
 // CHECK:bitcast i8 %{{.*}} to <8 x i1>
 // CHECK:select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}}
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:shufflevector <8 x double> %{{.*}}, <8 x double> undef, <4 x i32> 
-// CHECK:call <4 x double> @llvm.x86.avx.min.pd.256(<4 x double> %{{.*}}, <4 x double> %{{.*}})
-// CHECK:shufflevector <4 x double> %{{.*}}, <4 x double> undef, <2 x i32> 
-// CHECK:shufflevector <4 x double> %{{.*}}, <4 x double> undef, <2 x i32> 
-// CHECK:call <2 x double> @llvm.x86.sse2.min.pd(<2 x double> %{{.*}}, <2 x double> %{{.*}})
-// C

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Whatever the final decision is, maybe add a doxygen comment explaining the 
semantics?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93179

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

[PATCH] D93181: [NFC][AMDGPU] Reformat AMD GPU targets in cuda.cpp

[Tony Tye via Phabricator via cfe-commits]

t-tye created this revision.
t-tye added reviewers: kzhuravl, ronlieb.
Herald added subscribers: dexonsmith, tpr, dstuttard, yaxunl.
t-tye requested review of this revision.
Herald added subscribers: cfe-commits, wdng.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93181

Files:
  clang/lib/Basic/Cuda.cpp


Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -72,23 +72,34 @@
 SM(70), SM(72),  // Volta
 SM(75),  // Turing
 SM(80),  // Ampere
-GFX(600), // tahiti
-GFX(601), // pitcairn, verde
-GFX(602), // oland, hainan
-GFX(700), // kaveri
-GFX(701), // hawaii
-GFX(702), // 290,290x,R390,R390x
-GFX(703), // kabini mullins
-GFX(704), // bonaire
-GFX(705),
-GFX(801), // carrizo
-GFX(802), // tonga,iceland
-GFX(803), // fiji,polaris10
-GFX(805), // tongapro
-GFX(810), // stoney
-GFX(900), // vega, instinct
-GFX(902), GFX(904), GFX(906), GFX(908), GFX(909), GFX(90c),
-GFX(1010), GFX(1011), GFX(1012), GFX(1030), GFX(1031), GFX(1032), GFX(1033)
+GFX(600),  // gfx600
+GFX(601),  // gfx601
+GFX(602),  // gfx602
+GFX(700),  // gfx700
+GFX(701),  // gfx701
+GFX(702),  // gfx702
+GFX(703),  // gfx703
+GFX(704),  // gfx704
+GFX(705),  // gfx705
+GFX(801),  // gfx801
+GFX(802),  // gfx802
+GFX(803),  // gfx803
+GFX(805),  // gfx805
+GFX(810),  // gfx810
+GFX(900),  // gfx900
+GFX(902),  // gfx902
+GFX(904),  // gfx903
+GFX(906),  // gfx906
+GFX(908),  // gfx908
+GFX(909),  // gfx909
+GFX(90c),  // gfx90c
+GFX(1010), // gfx1010
+GFX(1011), // gfx1011
+GFX(1012), // gfx1012
+GFX(1030), // gfx1030
+GFX(1031), // gfx1031
+GFX(1032), // gfx1032
+GFX(1033), // gfx1033
 // clang-format on
 };
 #undef SM


Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -72,23 +72,34 @@
 SM(70), SM(72),  // Volta
 SM(75),  // Turing
 SM(80),  // Ampere
-GFX(600), // tahiti
-GFX(601), // pitcairn, verde
-GFX(602), // oland, hainan
-GFX(700), // kaveri
-GFX(701), // hawaii
-GFX(702), // 290,290x,R390,R390x
-GFX(703), // kabini mullins
-GFX(704), // bonaire
-GFX(705),
-GFX(801), // carrizo
-GFX(802), // tonga,iceland
-GFX(803), // fiji,polaris10
-GFX(805), // tongapro
-GFX(810), // stoney
-GFX(900), // vega, instinct
-GFX(902), GFX(904), GFX(906), GFX(908), GFX(909), GFX(90c),
-GFX(1010), GFX(1011), GFX(1012), GFX(1030), GFX(1031), GFX(1032), GFX(1033)
+GFX(600),  // gfx600
+GFX(601),  // gfx601
+GFX(602),  // gfx602
+GFX(700),  // gfx700
+GFX(701),  // gfx701
+GFX(702),  // gfx702
+GFX(703),  // gfx703
+GFX(704),  // gfx704
+GFX(705),  // gfx705
+GFX(801),  // gfx801
+GFX(802),  // gfx802
+GFX(803),  // gfx803
+GFX(805),  // gfx805
+GFX(810),  // gfx810
+GFX(900),  // gfx900
+GFX(902),  // gfx902
+GFX(904),  // gfx903
+GFX(906),  // gfx906
+GFX(908),  // gfx908
+GFX(909),  // gfx909
+GFX(90c),  // gfx90c
+GFX(1010), // gfx1010
+GFX(1011), // gfx1011
+GFX(1012), // gfx1012
+GFX(1030), // gfx1030
+GFX(1031), // gfx1031
+GFX(1032), // gfx1032
+GFX(1033), // gfx1033
 // clang-format on
 };
 #undef SM
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre added a comment.

In D92409#2450690 , @rsmith wrote:

> In D92409#2450550 , @thopre wrote:
>
>> In D92409#2426196 , @thopre wrote:
>>
>>> Is there a way to disable it from the header? I've noticed this warning 
>>> when compiling an application using libclang with g++. So I'm looking for a 
>>> fix outside clang's build system which is not used in that case.
>>
>> Ping?
>
> `#pragma GCC diagnostic ignored` 
> (https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html) can do it. Now, 
> we can't reasonably guarantee that Clang headers won't cause arbitrary broken 
> warnings to fire, so there's a judgment call here on the extent to which we 
> should carry changes to support use of such warning flags (versus expecting 
> the code including the header to disable the warning). In this case, it's 
> probably worth it, though, because the warning in question is part of GCC's 
> `-Wall`.

The documentation for that pragma says:

> Note that not all diagnostics are modifiable; at the moment only warnings 
> (normally controlled by ‘-W…’) can be controlled, and not all of them.

Unfortunately -Wcomment seems to be one of them as the pragma has no effect on 
the warning, at least with GCC 7, I haven't tried other versions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

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

[PATCH] D93181: [NFC][AMDGPU] Reformat AMD GPU targets in cuda.cpp

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl accepted this revision.
kzhuravl 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/D93181/new/

https://reviews.llvm.org/D93181

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

[PATCH] D78938: Make LLVM build in C++20 mode

[Nuno Lopes via Phabricator via cfe-commits]

nlopes added a comment.

Thanks @lebedev.ri for the pointer!
I started working on exactly the same thing as I was trying to link a C++20 
project with LLVM.
@BRevzin is there anything missing in this patch? Do you have commit access or 
do you need help to land this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D93182: [clang-tidy] Add linux kernel log functions checker

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added reviewers: nickdesaulniers, alexfh, hokein, aaron.ballman, 
njames93.
trixirt added a project: clang-tools-extra.
Herald added subscribers: Charusso, xazax.hun, mgorny.
trixirt requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The linux kernel's checkpatch.pl script does several checks
and fixits on the log functions.  This clang-tidy checker repeats
these checks so the fixits can be applied tree wide.

The first fixer looks for unneeded 'h' in format strings.

>From this reproducer

  printk("%hx\n", a);

checkpatch.pl produces this warning

WARNING: Integer promotion: Using 'h' in '%hx' is unnecessary
+  printk("%hx\n", a);

and this fix

- printk("%hx\n", a);

+  printk("%x\n", a);

LKML discussion
https://lore.kernel.org/lkml/5e3265c241602bb54286fbaae9222070daa4768e.ca...@perches.com/


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93182

Files:
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.h
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-logfunctions-h.c

Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-logfunctions-h.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-logfunctions-h.c
@@ -0,0 +1,20 @@
+// RUN: %check_clang_tidy %s linuxkernel-log-functions %t
+
+extern printk(const char *, ...) __attribute__((format(printf, 1, 2)));
+
+// scripts/checkpatch.pl produces
+// WARNING: Integer promotion: Using 'h' in '%hx' is unnecessary
+// ... linuxkernel-logfunctions-h.c:10:
+// +	printk("%hx\n", a);
+//
+// with --fix-inplace, the change is
+//  void f(unsigned char a)
+// {
+// -  printk("%hx\n", a);
+// +  printk("%x\n", a);
+// }
+void f(unsigned char a) {
+  printk("%hx\n", a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: Integer promotion: Using 'h' in '%hx' is unnecessay
+  // CHECK-FIXES: printk("%x\n", a);{{$}}
+}
Index: clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.h
@@ -0,0 +1,39 @@
+//===--- LogFunctionsCheck.h - clang-tidy ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_LOGFUNCTIONSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_LOGFUNCTIONSCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/Lex/MacroInfo.h"
+#include 
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+class LogFunctionsCheck : public ClangTidyCheck {
+
+  enum LogFunctionsFixerKind { LFFK_None, LFFK_H };
+
+public:
+  LogFunctionsCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  enum LogFunctionsFixerKind FixerKind;
+  const std::string LogFunctionsFixerKindName;
+};
+
+} // namespace linuxkernel
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_LOGFUNCTIONSCHECK_H
Index: clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.cpp
@@ -0,0 +1,95 @@
+//===--- LogFunctionsCheck.cpp - clang-tidy ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "LogFunctionsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+LogFunctionsCheck::LogFunctionsCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context), FixerKind(LFFK_H) {}
+
+void LogFunctionsCheck::registerMatchers(MatchFinder *Finder) {
+  if (FixerKind == LFFK_H) {
+// From the reproducer
+// extern int printk(const char *, ...) __attribute__((format(printf, 1, 2)));
+// void f(unsigned short a) { printk("%hx\n", a); 

[PATCH] D93185: [docs] Use make_unique in FrontendAction example

[Nicolás Alvarez via Phabricator via cfe-commits]

nicolas17 created this revision.
nicolas17 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The code example for "RecursiveASTVisitor based ASTFrontendActions"
was using unique_ptr(new X) when creating the AST consumer; change
it to use make_unique instead. The main function of the same example
already used make_unique.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93185

Files:
  clang/docs/RAVFrontendAction.rst


Index: clang/docs/RAVFrontendAction.rst
===
--- clang/docs/RAVFrontendAction.rst
+++ clang/docs/RAVFrontendAction.rst
@@ -27,8 +27,7 @@
   public:
 virtual std::unique_ptr CreateASTConsumer(
   clang::CompilerInstance &Compiler, llvm::StringRef InFile) {
-  return std::unique_ptr(
-  new FindNamedClassConsumer);
+  return std::make_unique();
 }
   };
 
@@ -114,8 +113,7 @@
 
   virtual std::unique_ptr CreateASTConsumer(
 clang::CompilerInstance &Compiler, llvm::StringRef InFile) {
-return std::unique_ptr(
-new FindNamedClassConsumer(&Compiler.getASTContext()));
+return 
std::make_unique(&Compiler.getASTContext());
   }
 
 Now that the ASTContext is available in the RecursiveASTVisitor, we can
@@ -189,8 +187,7 @@
   public:
 virtual std::unique_ptr CreateASTConsumer(
   clang::CompilerInstance &Compiler, llvm::StringRef InFile) {
-  return std::unique_ptr(
-  new FindNamedClassConsumer(&Compiler.getASTContext()));
+  return 
std::make_unique(&Compiler.getASTContext());
 }
   };
 


Index: clang/docs/RAVFrontendAction.rst
===
--- clang/docs/RAVFrontendAction.rst
+++ clang/docs/RAVFrontendAction.rst
@@ -27,8 +27,7 @@
   public:
 virtual std::unique_ptr CreateASTConsumer(
   clang::CompilerInstance &Compiler, llvm::StringRef InFile) {
-  return std::unique_ptr(
-  new FindNamedClassConsumer);
+  return std::make_unique();
 }
   };
 
@@ -114,8 +113,7 @@
 
   virtual std::unique_ptr CreateASTConsumer(
 clang::CompilerInstance &Compiler, llvm::StringRef InFile) {
-return std::unique_ptr(
-new FindNamedClassConsumer(&Compiler.getASTContext()));
+return std::make_unique(&Compiler.getASTContext());
   }
 
 Now that the ASTContext is available in the RecursiveASTVisitor, we can
@@ -189,8 +187,7 @@
   public:
 virtual std::unique_ptr CreateASTConsumer(
   clang::CompilerInstance &Compiler, llvm::StringRef InFile) {
-  return std::unique_ptr(
-  new FindNamedClassConsumer(&Compiler.getASTContext()));
+  return std::make_unique(&Compiler.getASTContext());
 }
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 5ad202c - [NFC][AMDGPU] Reformat AMD GPU targets in cuda.cpp

[via cfe-commits]

Author: Tony
Date: 2020-12-13T23:02:59Z
New Revision: 5ad202ce8963157242785a8345024a80c721ece2

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

LOG: [NFC][AMDGPU] Reformat AMD GPU targets in cuda.cpp

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

Added: 


Modified: 
clang/lib/Basic/Cuda.cpp

Removed: 




diff  --git a/clang/lib/Basic/Cuda.cpp b/clang/lib/Basic/Cuda.cpp
index d9f79a1a10d5..144113f2d2e7 100644
--- a/clang/lib/Basic/Cuda.cpp
+++ b/clang/lib/Basic/Cuda.cpp
@@ -72,23 +72,34 @@ CudaArchToStringMap arch_names[] = {
 SM(70), SM(72),  // Volta
 SM(75),  // Turing
 SM(80),  // Ampere
-GFX(600), // tahiti
-GFX(601), // pitcairn, verde
-GFX(602), // oland, hainan
-GFX(700), // kaveri
-GFX(701), // hawaii
-GFX(702), // 290,290x,R390,R390x
-GFX(703), // kabini mullins
-GFX(704), // bonaire
-GFX(705),
-GFX(801), // carrizo
-GFX(802), // tonga,iceland
-GFX(803), // fiji,polaris10
-GFX(805), // tongapro
-GFX(810), // stoney
-GFX(900), // vega, instinct
-GFX(902), GFX(904), GFX(906), GFX(908), GFX(909), GFX(90c),
-GFX(1010), GFX(1011), GFX(1012), GFX(1030), GFX(1031), GFX(1032), GFX(1033)
+GFX(600),  // gfx600
+GFX(601),  // gfx601
+GFX(602),  // gfx602
+GFX(700),  // gfx700
+GFX(701),  // gfx701
+GFX(702),  // gfx702
+GFX(703),  // gfx703
+GFX(704),  // gfx704
+GFX(705),  // gfx705
+GFX(801),  // gfx801
+GFX(802),  // gfx802
+GFX(803),  // gfx803
+GFX(805),  // gfx805
+GFX(810),  // gfx810
+GFX(900),  // gfx900
+GFX(902),  // gfx902
+GFX(904),  // gfx903
+GFX(906),  // gfx906
+GFX(908),  // gfx908
+GFX(909),  // gfx909
+GFX(90c),  // gfx90c
+GFX(1010), // gfx1010
+GFX(1011), // gfx1011
+GFX(1012), // gfx1012
+GFX(1030), // gfx1030
+GFX(1031), // gfx1031
+GFX(1032), // gfx1032
+GFX(1033), // gfx1033
 // clang-format on
 };
 #undef SM



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

[PATCH] D93181: [NFC][AMDGPU] Reformat AMD GPU targets in cuda.cpp

[Tony Tye via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5ad202ce8963: [NFC][AMDGPU] Reformat AMD GPU targets in 
cuda.cpp (authored by t-tye).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93181

Files:
  clang/lib/Basic/Cuda.cpp


Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -72,23 +72,34 @@
 SM(70), SM(72),  // Volta
 SM(75),  // Turing
 SM(80),  // Ampere
-GFX(600), // tahiti
-GFX(601), // pitcairn, verde
-GFX(602), // oland, hainan
-GFX(700), // kaveri
-GFX(701), // hawaii
-GFX(702), // 290,290x,R390,R390x
-GFX(703), // kabini mullins
-GFX(704), // bonaire
-GFX(705),
-GFX(801), // carrizo
-GFX(802), // tonga,iceland
-GFX(803), // fiji,polaris10
-GFX(805), // tongapro
-GFX(810), // stoney
-GFX(900), // vega, instinct
-GFX(902), GFX(904), GFX(906), GFX(908), GFX(909), GFX(90c),
-GFX(1010), GFX(1011), GFX(1012), GFX(1030), GFX(1031), GFX(1032), GFX(1033)
+GFX(600),  // gfx600
+GFX(601),  // gfx601
+GFX(602),  // gfx602
+GFX(700),  // gfx700
+GFX(701),  // gfx701
+GFX(702),  // gfx702
+GFX(703),  // gfx703
+GFX(704),  // gfx704
+GFX(705),  // gfx705
+GFX(801),  // gfx801
+GFX(802),  // gfx802
+GFX(803),  // gfx803
+GFX(805),  // gfx805
+GFX(810),  // gfx810
+GFX(900),  // gfx900
+GFX(902),  // gfx902
+GFX(904),  // gfx903
+GFX(906),  // gfx906
+GFX(908),  // gfx908
+GFX(909),  // gfx909
+GFX(90c),  // gfx90c
+GFX(1010), // gfx1010
+GFX(1011), // gfx1011
+GFX(1012), // gfx1012
+GFX(1030), // gfx1030
+GFX(1031), // gfx1031
+GFX(1032), // gfx1032
+GFX(1033), // gfx1033
 // clang-format on
 };
 #undef SM


Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -72,23 +72,34 @@
 SM(70), SM(72),  // Volta
 SM(75),  // Turing
 SM(80),  // Ampere
-GFX(600), // tahiti
-GFX(601), // pitcairn, verde
-GFX(602), // oland, hainan
-GFX(700), // kaveri
-GFX(701), // hawaii
-GFX(702), // 290,290x,R390,R390x
-GFX(703), // kabini mullins
-GFX(704), // bonaire
-GFX(705),
-GFX(801), // carrizo
-GFX(802), // tonga,iceland
-GFX(803), // fiji,polaris10
-GFX(805), // tongapro
-GFX(810), // stoney
-GFX(900), // vega, instinct
-GFX(902), GFX(904), GFX(906), GFX(908), GFX(909), GFX(90c),
-GFX(1010), GFX(1011), GFX(1012), GFX(1030), GFX(1031), GFX(1032), GFX(1033)
+GFX(600),  // gfx600
+GFX(601),  // gfx601
+GFX(602),  // gfx602
+GFX(700),  // gfx700
+GFX(701),  // gfx701
+GFX(702),  // gfx702
+GFX(703),  // gfx703
+GFX(704),  // gfx704
+GFX(705),  // gfx705
+GFX(801),  // gfx801
+GFX(802),  // gfx802
+GFX(803),  // gfx803
+GFX(805),  // gfx805
+GFX(810),  // gfx810
+GFX(900),  // gfx900
+GFX(902),  // gfx902
+GFX(904),  // gfx903
+GFX(906),  // gfx906
+GFX(908),  // gfx908
+GFX(909),  // gfx909
+GFX(90c),  // gfx90c
+GFX(1010), // gfx1010
+GFX(1011), // gfx1011
+GFX(1012), // gfx1012
+GFX(1030), // gfx1030
+GFX(1031), // gfx1031
+GFX(1032), // gfx1032
+GFX(1033), // gfx1033
 // clang-format on
 };
 #undef SM
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91025: [clangd] Fix locateMacroAt() for macro definition outside preamble

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 311472.
nridge marked 3 inline comments as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91025

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1624,6 +1624,14 @@
 }
   )cpp",
 
+  R"cpp(// Macro outside preamble
+int breakPreamble;
+#define [[MA^CRO]](X) (X+1)
+void test() {
+  int x = [[MACRO]]([[MACRO]](1));
+}
+  )cpp",
+
   R"cpp(
 int [[v^ar]] = 0;
 void foo(int s = [[var]]);
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -975,17 +975,30 @@
   if (!IdentifierInfo || !IdentifierInfo->hadMacroDefinition())
 return None;
 
-  // Get the definition just before the searched location so that a macro
-  // referenced in a '#undef MACRO' can still be found. Note that we only do
-  // that if Loc is not pointing at start of file.
-  if (SM.getLocForStartOfFile(SM.getFileID(Loc)) != Loc)
-Loc = Loc.getLocWithOffset(-1);
-  MacroDefinition MacroDef = PP.getMacroDefinitionAtLoc(IdentifierInfo, Loc);
-  if (auto *MI = MacroDef.getMacroInfo())
-return DefinedMacro{
-IdentifierInfo->getName(), MI,
-translatePreamblePatchLocation(MI->getDefinitionLoc(), SM)};
-  return None;
+  // We need to take special case to handle #define and #undef.
+  // Preprocessor::getMacroDefinitionAtLoc() only considers a macro
+  // definition to be in scope *after* the location of the macro name in a
+  // #define that introduces it, and *before* the location of the macro name
+  // in an #undef that undefines it. To handle these cases, we check for
+  // the macro being in scope either just after or just before the location
+  // of the token. In getting the location before, we also take care to check
+  // for start-of-file.
+  FileID FID = SM.getFileID(Loc);
+  assert(Loc != SM.getLocForEndOfFile(FID));
+  SourceLocation JustAfterToken = Loc.getLocWithOffset(1);
+  auto *MacroInfo =
+  PP.getMacroDefinitionAtLoc(IdentifierInfo, 
JustAfterToken).getMacroInfo();
+  if (!MacroInfo && SM.getLocForStartOfFile(FID) != Loc) {
+SourceLocation JustBeforeToken = Loc.getLocWithOffset(-1);
+MacroInfo = PP.getMacroDefinitionAtLoc(IdentifierInfo, JustBeforeToken)
+.getMacroInfo();
+  }
+  if (!MacroInfo) {
+return None;
+  }
+  return DefinedMacro{
+  IdentifierInfo->getName(), MacroInfo,
+  translatePreamblePatchLocation(MacroInfo->getDefinitionLoc(), SM)};
 }
 
 llvm::Expected Edit::apply() const {


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1624,6 +1624,14 @@
 }
   )cpp",
 
+  R"cpp(// Macro outside preamble
+int breakPreamble;
+#define [[MA^CRO]](X) (X+1)
+void test() {
+  int x = [[MACRO]]([[MACRO]](1));
+}
+  )cpp",
+
   R"cpp(
 int [[v^ar]] = 0;
 void foo(int s = [[var]]);
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -975,17 +975,30 @@
   if (!IdentifierInfo || !IdentifierInfo->hadMacroDefinition())
 return None;
 
-  // Get the definition just before the searched location so that a macro
-  // referenced in a '#undef MACRO' can still be found. Note that we only do
-  // that if Loc is not pointing at start of file.
-  if (SM.getLocForStartOfFile(SM.getFileID(Loc)) != Loc)
-Loc = Loc.getLocWithOffset(-1);
-  MacroDefinition MacroDef = PP.getMacroDefinitionAtLoc(IdentifierInfo, Loc);
-  if (auto *MI = MacroDef.getMacroInfo())
-return DefinedMacro{
-IdentifierInfo->getName(), MI,
-translatePreamblePatchLocation(MI->getDefinitionLoc(), SM)};
-  return None;
+  // We need to take special case to handle #define and #undef.
+  // Preprocessor::getMacroDefinitionAtLoc() only considers a macro
+  // definition to be in scope *after* the location of the macro name in a
+  // #define that introduces it, and *before* the location of the macro name
+  // in an #undef that undefines it. To handle these cases, we check for
+  // the macro being in scope either just after or just before the location
+  // of the token. In getting th

[clang-tools-extra] fef242c - [clangd] Fix locateMacroAt() for macro definition outside preamble

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2020-12-13T18:33:33-05:00
New Revision: fef242c32e833b84e8b46bd56a28c01c5f9aa65d

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

LOG: [clangd] Fix locateMacroAt() for macro definition outside preamble

Fixes https://github.com/clangd/clangd/issues/577

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

Added: 


Modified: 
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SourceCode.cpp 
b/clang-tools-extra/clangd/SourceCode.cpp
index 54248386d7b4..c0ccf2152750 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -975,17 +975,30 @@ llvm::Optional locateMacroAt(const 
syntax::Token &SpelledTok,
   if (!IdentifierInfo || !IdentifierInfo->hadMacroDefinition())
 return None;
 
-  // Get the definition just before the searched location so that a macro
-  // referenced in a '#undef MACRO' can still be found. Note that we only do
-  // that if Loc is not pointing at start of file.
-  if (SM.getLocForStartOfFile(SM.getFileID(Loc)) != Loc)
-Loc = Loc.getLocWithOffset(-1);
-  MacroDefinition MacroDef = PP.getMacroDefinitionAtLoc(IdentifierInfo, Loc);
-  if (auto *MI = MacroDef.getMacroInfo())
-return DefinedMacro{
-IdentifierInfo->getName(), MI,
-translatePreamblePatchLocation(MI->getDefinitionLoc(), SM)};
-  return None;
+  // We need to take special case to handle #define and #undef.
+  // Preprocessor::getMacroDefinitionAtLoc() only considers a macro
+  // definition to be in scope *after* the location of the macro name in a
+  // #define that introduces it, and *before* the location of the macro name
+  // in an #undef that undefines it. To handle these cases, we check for
+  // the macro being in scope either just after or just before the location
+  // of the token. In getting the location before, we also take care to check
+  // for start-of-file.
+  FileID FID = SM.getFileID(Loc);
+  assert(Loc != SM.getLocForEndOfFile(FID));
+  SourceLocation JustAfterToken = Loc.getLocWithOffset(1);
+  auto *MacroInfo =
+  PP.getMacroDefinitionAtLoc(IdentifierInfo, 
JustAfterToken).getMacroInfo();
+  if (!MacroInfo && SM.getLocForStartOfFile(FID) != Loc) {
+SourceLocation JustBeforeToken = Loc.getLocWithOffset(-1);
+MacroInfo = PP.getMacroDefinitionAtLoc(IdentifierInfo, JustBeforeToken)
+.getMacroInfo();
+  }
+  if (!MacroInfo) {
+return None;
+  }
+  return DefinedMacro{
+  IdentifierInfo->getName(), MacroInfo,
+  translatePreamblePatchLocation(MacroInfo->getDefinitionLoc(), SM)};
 }
 
 llvm::Expected Edit::apply() const {

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index c3c87bd628bd..5b0ceb1cc200 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1624,6 +1624,14 @@ TEST(FindReferences, WithinAST) {
 }
   )cpp",
 
+  R"cpp(// Macro outside preamble
+int breakPreamble;
+#define [[MA^CRO]](X) (X+1)
+void test() {
+  int x = [[MACRO]]([[MACRO]](1));
+}
+  )cpp",
+
   R"cpp(
 int [[v^ar]] = 0;
 void foo(int s = [[var]]);



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

[PATCH] D91025: [clangd] Fix locateMacroAt() for macro definition outside preamble

[Nathan Ridge via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfef242c32e83: [clangd] Fix locateMacroAt() for macro 
definition outside preamble (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91025

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1624,6 +1624,14 @@
 }
   )cpp",
 
+  R"cpp(// Macro outside preamble
+int breakPreamble;
+#define [[MA^CRO]](X) (X+1)
+void test() {
+  int x = [[MACRO]]([[MACRO]](1));
+}
+  )cpp",
+
   R"cpp(
 int [[v^ar]] = 0;
 void foo(int s = [[var]]);
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -975,17 +975,30 @@
   if (!IdentifierInfo || !IdentifierInfo->hadMacroDefinition())
 return None;
 
-  // Get the definition just before the searched location so that a macro
-  // referenced in a '#undef MACRO' can still be found. Note that we only do
-  // that if Loc is not pointing at start of file.
-  if (SM.getLocForStartOfFile(SM.getFileID(Loc)) != Loc)
-Loc = Loc.getLocWithOffset(-1);
-  MacroDefinition MacroDef = PP.getMacroDefinitionAtLoc(IdentifierInfo, Loc);
-  if (auto *MI = MacroDef.getMacroInfo())
-return DefinedMacro{
-IdentifierInfo->getName(), MI,
-translatePreamblePatchLocation(MI->getDefinitionLoc(), SM)};
-  return None;
+  // We need to take special case to handle #define and #undef.
+  // Preprocessor::getMacroDefinitionAtLoc() only considers a macro
+  // definition to be in scope *after* the location of the macro name in a
+  // #define that introduces it, and *before* the location of the macro name
+  // in an #undef that undefines it. To handle these cases, we check for
+  // the macro being in scope either just after or just before the location
+  // of the token. In getting the location before, we also take care to check
+  // for start-of-file.
+  FileID FID = SM.getFileID(Loc);
+  assert(Loc != SM.getLocForEndOfFile(FID));
+  SourceLocation JustAfterToken = Loc.getLocWithOffset(1);
+  auto *MacroInfo =
+  PP.getMacroDefinitionAtLoc(IdentifierInfo, 
JustAfterToken).getMacroInfo();
+  if (!MacroInfo && SM.getLocForStartOfFile(FID) != Loc) {
+SourceLocation JustBeforeToken = Loc.getLocWithOffset(-1);
+MacroInfo = PP.getMacroDefinitionAtLoc(IdentifierInfo, JustBeforeToken)
+.getMacroInfo();
+  }
+  if (!MacroInfo) {
+return None;
+  }
+  return DefinedMacro{
+  IdentifierInfo->getName(), MacroInfo,
+  translatePreamblePatchLocation(MacroInfo->getDefinitionLoc(), SM)};
 }
 
 llvm::Expected Edit::apply() const {


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1624,6 +1624,14 @@
 }
   )cpp",
 
+  R"cpp(// Macro outside preamble
+int breakPreamble;
+#define [[MA^CRO]](X) (X+1)
+void test() {
+  int x = [[MACRO]]([[MACRO]](1));
+}
+  )cpp",
+
   R"cpp(
 int [[v^ar]] = 0;
 void foo(int s = [[var]]);
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -975,17 +975,30 @@
   if (!IdentifierInfo || !IdentifierInfo->hadMacroDefinition())
 return None;
 
-  // Get the definition just before the searched location so that a macro
-  // referenced in a '#undef MACRO' can still be found. Note that we only do
-  // that if Loc is not pointing at start of file.
-  if (SM.getLocForStartOfFile(SM.getFileID(Loc)) != Loc)
-Loc = Loc.getLocWithOffset(-1);
-  MacroDefinition MacroDef = PP.getMacroDefinitionAtLoc(IdentifierInfo, Loc);
-  if (auto *MI = MacroDef.getMacroInfo())
-return DefinedMacro{
-IdentifierInfo->getName(), MI,
-translatePreamblePatchLocation(MI->getDefinitionLoc(), SM)};
-  return None;
+  // We need to take special case to handle #define and #undef.
+  // Preprocessor::getMacroDefinitionAtLoc() only considers a macro
+  // definition to be in scope *after* the location of the macro name in a
+  // #define that introduces it, and *before* the location of the macro name
+  // in an #undef that undefines it. To handle these cases, we che

[PATCH] D92290: [clangd] Use clangd's Context mechanism to make the ASTContext of the AST being operated on available everywhere

[Nathan Ridge via Phabricator via cfe-commits]

nridge added reviewers: sammccall, hokein, kadircet.
nridge added a comment.

Shopping the patch around to some possible reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92290

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

[PATCH] D93182: [clang-tidy] Add linux kernel log functions checker

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

This check needs documentation adding in 
`clang-tools-extra/docs/clang-tidy/checks/linux-kernel-logfunctions-h.rst`
Also needs to add this in `clang-tools-extra/docs/clang-tidy/checks/list.rst`, 
keeping it in alphabetical order.
Also need to add an entry to `clang-tools-extra/docs/ReleaseNotes.rst`

You may also need to check the state of your local branch as the pre merge 
build bot can't apply this cleanly in order to test.




Comment at: clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.cpp:42-45
+compoundStmt(has(
+callExpr(has(ignoringParenImpCasts(stringLiteral().bind("lit"))),
+ callee(functionDecl(hasAttr(attr::Format)).bind("func")))
+.bind("call"))),

Why are you matching on the `compoundStmt`, surely the `callExpr` should be the 
top level matcher?
Doing this will miss cases where 2 occurance of `printf` occur in the same 
`compoundStmt` and miss the cases where `printf` isn't in a `compoundStmt`.
```lang=c
if (...)
  printf(...);```



Comment at: clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.cpp:58
+if (Format->getType()->getName() == "printf" &&
+ArgNum < Call->getNumArgs() &&
+Literal == Call->getArg(ArgNum)->IgnoreCasts() && Literal->isAscii() &&

Should this be an assert, I can't think of a reason why the FormatIDx would be 
out of range.



Comment at: clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.cpp:59
+ArgNum < Call->getNumArgs() &&
+Literal == Call->getArg(ArgNum)->IgnoreCasts() && Literal->isAscii() &&
+Literal->getCharByteWidth() == 1) {

Why bother binding the `lit` node if you can just get it from the FormatArgs. 
This is also potentially safer also as it won't get confused by
```lang=c
printf("fmtString", "formatArg");```



Comment at: clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.cpp:64
+  "%hhi", "%hhd", "%hhu", "%hhx"};
+  for (auto FoundH : LookForH) {
+if (LiteralString.contains(FoundH)) {

This whole loop looks wrong.
It will only report the first instance of `FoundH` and its doing a lot of 
duplicated work
It would be better to scan across the string for `%h` then check the next chars 
for `[idux]` or `h[idux]`



Comment at: clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.cpp:65
+  for (auto FoundH : LookForH) {
+if (LiteralString.contains(FoundH)) {
+  size_t HBegin = LiteralString.find(FoundH) + 1;

Can this be refactored to use an early exit instead.



Comment at: clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.cpp:79
+  Result.Context->getLangOpts(), Result.Context->getTargetInfo());
+  diag(EndLoc, "Integer promotion: Using '%0' in '%1' is unnecessay")
+  << ErrorString << FoundH

Why is the diag location reported as the end of the format specifier, It should 
really be pointed to the start of the format specifier or location of the `h`.
```lang=c
printf("%hx, a);
^
printf("%hx, a);
 ^
``` 




Comment at: clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.cpp:90
+void LogFunctionsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "Fixer", "");
+}

There is no option retrieved called `Fixer`, so there should be no option 
stored call `Fixer`.



Comment at: clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.h:13-14
+#include "../ClangTidyCheck.h"
+#include "clang/Lex/MacroInfo.h"
+#include 
+

These includes seem redundant in this file.



Comment at: clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.h:22
+
+  enum LogFunctionsFixerKind { LFFK_None, LFFK_H };
+

Right now you are only using `LFFK_H`. So for simplicity, this enum should be 
removed from this patch. If there is a follow up where this actually used, the 
enum should be added there.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/linuxkernel-logfunctions-h.c:3
+
+extern printk(const char *, ...) __attribute__((format(printf, 1, 2)));
+

Can you add a check with a function
```lang=c
extern printf(const char *, ...);
```
Just to demonstrate it will only flag functions that have the printf format 
attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93182

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

[PATCH] D92290: [clangd] Use clangd's Context mechanism to make the ASTContext of the AST being operated on available everywhere

[Nathan Ridge via Phabricator via cfe-commits]

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

Add a missed call site


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92290

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h

Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -76,6 +76,10 @@
   // to eliminate this option some day.
   bool OverlayRealFileSystemForModules = false;
 
+  // This is used to add the ASTContext of the AST produced by build() to the
+  // clangd Context.
+  mutable std::unique_ptr ASTCtx;
+
   // By default, build() will report Error diagnostics as GTest errors.
   // Suppress this behavior by adding an 'error-ok' comment to the code.
   ParsedAST build() const;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -139,6 +139,8 @@
 break; // Just report first error for simplicity.
   }
   }
+  ASTCtx = std::make_unique(ParsedAST::CurrentASTContext,
+  &AST->getASTContext());
   return std::move(*AST);
 }
 
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -721,6 +721,8 @@
   return Action(error(llvm::errc::invalid_argument, "invalid AST"));
 vlog("ASTWorker running {0} on version {2} of {1}", Name, FileName,
  FileInputs.Version);
+WithContextValue Ctx(ParsedAST::CurrentASTContext,
+ &(*AST)->getASTContext());
 Action(InputsAndAST{FileInputs, **AST});
   };
   startTask(Name, std::move(Task), /*UpdateType=*/None, Invalidation);
Index: clang-tools-extra/clangd/ParsedAST.h
===
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -26,6 +26,7 @@
 #include "Headers.h"
 #include "Preamble.h"
 #include "index/CanonicalIncludes.h"
+#include "support/Context.h"
 #include "support/Path.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
@@ -109,6 +110,10 @@
   /// AST. Might be None if no Preamble is used.
   llvm::Optional preambleVersion() const;
 
+  /// This context variable makes available the ASTContext of the AST being
+  /// operated on currently.
+  static Key CurrentASTContext;
+
 private:
   ParsedAST(llvm::StringRef Version,
 std::shared_ptr Preamble,
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -237,6 +237,8 @@
 
 } // namespace
 
+Key ParsedAST::CurrentASTContext;
+
 llvm::Optional
 ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
  std::unique_ptr CI,
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -8,6 +8,8 @@
 
 #include "FindTarget.h"
 #include "AST.h"
+#include "ParsedAST.h"
+#include "support/Context.h"
 #include "support/Logger.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
@@ -98,21 +100,18 @@
 // name (e.g. if it's an operator name), but the caller may not have
 // access to an ASTContext.
 std::vector getMembersReferencedViaDependentName(
-const Type *T,
-llvm::function_ref NameFactory,
+const Type *T, DeclarationName Name,
 llvm::function_ref Filter) {
   if (!T)
 return {};
   if (auto *ET = T->getAs()) {
-auto Result =
-ET->getDecl()->lookup(NameFactory(ET->getDecl()->getASTContext()));
+auto Result = ET->getDecl()->lookup(Name);
 return {Result.begin(), Result.end()};
   }
   if (auto *RD = resolveTypeToRecordDecl(T)) {
 if (!RD->hasDefinition())
   return {};
 RD = RD->getDefinition();
-DeclarationName Name = NameFactory(RD->getASTContext());
 return RD->lookupDependentName(Name, Filter);
   }
   return {};
@@ -147,9 +146,9 @@
   // smart pointer type.
   auto ArrowOps = getMembersReferencedViaDependentName(
   T,
-  [](ASTContext &Ctx) {
-return Ctx.DeclarationNames.getCXXOperatorName(OO_Arrow);
-  },
+  Context::current()
+  .getExisting(ParsedAST::C

Re: Buildbot to listen main branch

[Galina Kistanova via cfe-commits]

Hello everyone,

The buildbot listens to the "main" branch now.

Please let me know if you will see issues.

Thanks

Galina


On Tue, Dec 8, 2020 at 9:57 PM Galina Kistanova 
wrote:

> Hello everyone,
>
> To follow the main branch creation I'm going to update the buildbot to
> listen for the changes in this branch and instruct all the workers to
> checkout this branch.
>
> No work is required on the workers.
>
> Please check your annotated scripts to make sure it accepts the branch
> name from the buildbot, or change it if that's hardcoded.
>
> The staging has been updated today. I forced clean builds just to play it
> safe. Please check your bots in the staging area, to make sure everything
> is good.
>
> If everything will go well, the production buildbot will be updated on
> this Saturday, December, 12th.
>
> Thanks
>
> Galina
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D91245#2437518 , @MyDeveloperDay 
wrote:

> What does `the behavior goes wrong` mean? why can't it be left aligned?

Because when we use PointerAlignment attribute, I think we mean that the 
pointer need to be aligned in declarations. However,  `co_return *a;` is not a 
declaration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91245

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

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 311485.
ChuanqiXu added a comment.

Re-upload patch with context


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

https://reviews.llvm.org/D91245

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7755,6 +7755,15 @@
 
   verifyFormat("co_yield -1;");
   verifyFormat("co_return -1;");
+
+  // The default setting for PointerAlignment is PAS_Right.
+  // But if we set PointerAlignment as PAS_Left, the formatter
+  // would mis-format the pointer alignment.
+  FormatStyle PASLeftStyle = getLLVMStyle();
+  PASLeftStyle.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("co_return *a;", PASLeftStyle);
+  verifyFormat("co_await *a;", PASLeftStyle);
+  verifyFormat("co_yield *a", PASLeftStyle);
 }
 
 TEST_F(FormatTest, DoesNotIndentRelativeToUnaryOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1960,6 +1960,7 @@
 
 if (PrevToken->isOneOf(tok::l_paren, tok::l_square, tok::l_brace,
tok::comma, tok::semi, tok::kw_return, tok::colon,
+   tok::kw_co_return, tok::kw_co_await, 
tok::kw_co_yield,
tok::equal, tok::kw_delete, tok::kw_sizeof,
tok::kw_throw) ||
 PrevToken->isOneOf(TT_BinaryOperator, TT_ConditionalExpr,


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7755,6 +7755,15 @@
 
   verifyFormat("co_yield -1;");
   verifyFormat("co_return -1;");
+
+  // The default setting for PointerAlignment is PAS_Right.
+  // But if we set PointerAlignment as PAS_Left, the formatter
+  // would mis-format the pointer alignment.
+  FormatStyle PASLeftStyle = getLLVMStyle();
+  PASLeftStyle.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("co_return *a;", PASLeftStyle);
+  verifyFormat("co_await *a;", PASLeftStyle);
+  verifyFormat("co_yield *a", PASLeftStyle);
 }
 
 TEST_F(FormatTest, DoesNotIndentRelativeToUnaryOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1960,6 +1960,7 @@
 
 if (PrevToken->isOneOf(tok::l_paren, tok::l_square, tok::l_brace,
tok::comma, tok::semi, tok::kw_return, tok::colon,
+   tok::kw_co_return, tok::kw_co_await, tok::kw_co_yield,
tok::equal, tok::kw_delete, tok::kw_sizeof,
tok::kw_throw) ||
 PrevToken->isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93182: [clang-tidy] Add linux kernel log functions checker

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Maybe Clang's `-Wformat` should be extended to cover Linux kernel-specific 
format options?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93182

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

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D91245#2437741 , @Quuxplusone wrote:

> Peanut gallery says: It seems to me that your root problem here is that 
> `co_yield` is not being recognized by the parser as a keyword, and so you're 
> seeing e.g. `co_yield* p;` the same way you'd see `CoYield* p;`.
> But in that case, you should also be seeing `co_yield -1;` formatted as 
> `co_yield - 1;` the same way you'd see `CoYield - 1;`.
> Is it possible that you're not using the right (C++20) language mode? (OTOH, 
> see D69144 , D69180 
> , which //also// didn't concern themselves 
> with language modes. I don't understand why not.)
>
> Could you please re-upload the diff with context (i.e. `git show -U999`)?

Yes, it looks like that `co_yield` is not being recognized as keywords. I use 
the `Auto` for Standard attribute in `.clang-format` config file. And `co*` 
would be recognized as keywords. From my point of view, I first find that the 
keyword `return`  was processed specifically in the line 1962 in 
`TokenAnnotator.cpp` which recognize `return` as Unary Operator. So I choose to 
recognize `co*` keywords as Unary Operators to solve the format problem.


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

https://reviews.llvm.org/D91245

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

[clang] abbd57e - Factor out and centralize repeated 'getExpandedPackSize'.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-12-13T22:43:23-08:00
New Revision: abbd57e558b907a7be8adc5a5b9699dd7c23b1af

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

LOG: Factor out and centralize repeated 'getExpandedPackSize'.

Added: 


Modified: 
clang/include/clang/AST/DeclTemplate.h
clang/lib/AST/DeclTemplate.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index 641647659c17..7fbf6294970e 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -3353,6 +3353,36 @@ inline TemplateDecl *getAsTypeTemplateDecl(Decl *D) {
  : nullptr;
 }
 
+/// Check whether the template parameter is a pack expansion, and if so,
+/// determine the number of parameters produced by that expansion. For 
instance:
+///
+/// \code
+/// template struct A {
+///   template class ...TTs, typename ...Us> struct B;
+/// };
+/// \endcode
+///
+/// In \c A::B, \c NTs and \c TTs have expanded pack size 2, and \c Us
+/// is not a pack expansion, so returns an empty Optional.
+inline Optional getExpandedPackSize(const NamedDecl *Param) {
+  if (const auto *TTP = dyn_cast(Param)) {
+if (TTP->isExpandedParameterPack())
+  return TTP->getNumExpansionParameters();
+  }
+
+  if (const auto *NTTP = dyn_cast(Param)) {
+if (NTTP->isExpandedParameterPack())
+  return NTTP->getNumExpansionTypes();
+  }
+
+  if (const auto *TTP = dyn_cast(Param)) {
+if (TTP->isExpandedParameterPack())
+  return TTP->getNumExpansionTemplateParameters();
+  }
+
+  return None;
+}
+
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_DECLTEMPLATE_H

diff  --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 328ceaa63df3..25235c56ec46 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -102,24 +102,10 @@ unsigned TemplateParameterList::getMinRequiredArguments() 
const {
   unsigned NumRequiredArgs = 0;
   for (const NamedDecl *P : asArray()) {
 if (P->isTemplateParameterPack()) {
-  if (const auto *NTTP = dyn_cast(P)) {
-if (NTTP->isExpandedParameterPack()) {
-  NumRequiredArgs += NTTP->getNumExpansionTypes();
-  continue;
-}
-  } else if (const auto *TTP = dyn_cast(P)) {
-if (TTP->isExpandedParameterPack()) {
-  NumRequiredArgs += TTP->getNumExpansionParameters();
-  continue;
-}
-  } else {
-const auto *TP = cast(P);
-if (TP->isExpandedParameterPack()) {
-  NumRequiredArgs += TP->getNumExpansionTemplateParameters();
-  continue;
-}
+  if (Optional Expansions = getExpandedPackSize(P)) {
+NumRequiredArgs += *Expansions;
+continue;
   }
-
   break;
 }
 

diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 4176aa1f458f..70a25fb782e9 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -5588,39 +5588,6 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param,
   return false;
 }
 
-/// Check whether the template parameter is a pack expansion, and if so,
-/// determine the number of parameters produced by that expansion. For 
instance:
-///
-/// \code
-/// template struct A {
-///   template class ...TTs, typename ...Us> struct B;
-/// };
-/// \endcode
-///
-/// In \c A::B, \c NTs and \c TTs have expanded pack size 2, and \c Us
-/// is not a pack expansion, so returns an empty Optional.
-static Optional getExpandedPackSize(NamedDecl *Param) {
-  if (TemplateTypeParmDecl *TTP
-= dyn_cast(Param)) {
-if (TTP->isExpandedParameterPack())
-  return TTP->getNumExpansionParameters();
-  }
-
-  if (NonTypeTemplateParmDecl *NTTP
-= dyn_cast(Param)) {
-if (NTTP->isExpandedParameterPack())
-  return NTTP->getNumExpansionTypes();
-  }
-
-  if (TemplateTemplateParmDecl *TTP
-= dyn_cast(Param)) {
-if (TTP->isExpandedParameterPack())
-  return TTP->getNumExpansionTemplateParameters();
-  }
-
-  return None;
-}
-
 /// Diagnose a missing template argument.
 template
 static bool diagnoseMissingArgument(Sema &S, SourceLocation Loc,

diff  --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index d137aec82b8c..4a3b64cf5425 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -658,23 +658,6 @@ static TemplateParameter makeTemplateParameter(Decl *D) {
   return TemplateParameter(cast(D));
 }
 
-/// If \p Param is an expanded parameter pack, get the number of expansions.
-static Optional getExpandedPackSize(NamedDecl *Param) {
-  if (auto *TTP =

[clang] 05cdf4a - Consider reference, pointer, and pointer-to-member TemplateArguments to be different if they have different types.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-12-13T22:43:24-08:00
New Revision: 05cdf4acf42acce9ddcff646a5d6ac666710fe6d

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

LOG: Consider reference, pointer, and pointer-to-member TemplateArguments to be 
different if they have different types.

For the Itanium ABI, this implements the mangling rule suggested in
https://github.com/itanium-cxx-abi/cxx-abi/issues/47, namely mangling
such template arguments as being cast to the parameter type in the case
where the template name is overloadable. This can cause a mangling
change for rare cases, where

 * the template argument declaration is converted from its declared type
   to the type of the template parameter, and
 * the template parameter either has a deduced type or is a parameter of
   a function template.

However, such changes are necessary to avoid mangling collisions. The
ABI changes can be reversed with -fclang-abi-compat=11 or earlier.

Re-commit with a fix for the regression introduced last time: don't
expect parameters and arguments to line up inside an 
mangling.

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

Added: 


Modified: 
clang/include/clang/Basic/LangOptions.h
clang/lib/AST/ItaniumMangle.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/AST/TemplateBase.cpp
clang/test/CodeGenCXX/clang-abi-compat.cpp
clang/test/CodeGenCXX/mangle-class-nttp.cpp
clang/test/CodeGenCXX/mangle-template.cpp
clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index 203c45fdd9a7..251c9a9ecb5d 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -155,8 +155,10 @@ class LangOptions : public LangOptionsBase {
 
 /// Attempt to be ABI-compatible with code generated by Clang 11.0.x
 /// (git  2e10b7a39b93). This causes clang to pass unions with a 256-bit
-/// vector member on the stack instead of using registers, and to not
-/// properly mangle substitutions for template names in some cases.
+/// vector member on the stack instead of using registers, to not properly
+/// mangle substitutions for template names in some cases, and to mangle
+/// declaration template arguments without a cast to the parameter type
+/// even when that can lead to mangling collisions.
 Ver11,
 
 /// Conform to the underlying platform's C and C++ ABIs as closely

diff  --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index f5a4f6708c83..fe4968052e17 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -551,13 +551,15 @@ class CXXNameMangler {
   void mangleCXXCtorType(CXXCtorType T, const CXXRecordDecl *InheritedFrom);
   void mangleCXXDtorType(CXXDtorType T);
 
-  void mangleTemplateArgs(const TemplateArgumentLoc *TemplateArgs,
+  void mangleTemplateArgs(TemplateName TN,
+  const TemplateArgumentLoc *TemplateArgs,
   unsigned NumTemplateArgs);
-  void mangleTemplateArgs(const TemplateArgument *TemplateArgs,
+  void mangleTemplateArgs(TemplateName TN, const TemplateArgument 
*TemplateArgs,
   unsigned NumTemplateArgs);
-  void mangleTemplateArgs(const TemplateArgumentList &AL);
-  void mangleTemplateArg(TemplateArgument A);
-  void mangleValueInTemplateArg(QualType T, const APValue &V);
+  void mangleTemplateArgs(TemplateName TN, const TemplateArgumentList &AL);
+  void mangleTemplateArg(TemplateArgument A, bool NeedExactType);
+  void mangleValueInTemplateArg(QualType T, const APValue &V, bool TopLevel,
+bool NeedExactType = false);
 
   void mangleTemplateParameter(unsigned Depth, unsigned Index);
 
@@ -823,6 +825,11 @@ isTemplate(GlobalDecl GD, const TemplateArgumentList 
*&TemplateArgs) {
   return GlobalDecl();
 }
 
+static TemplateName asTemplateName(GlobalDecl GD) {
+  const TemplateDecl *TD = dyn_cast_or_null(GD.getDecl());
+  return TemplateName(const_cast(TD));
+}
+
 void CXXNameMangler::mangleName(GlobalDecl GD) {
   const NamedDecl *ND = cast(GD.getDecl());
   if (const VarDecl *VD = dyn_cast(ND)) {
@@ -899,7 +906,7 @@ void CXXNameMangler::mangleNameWithAbiTags(GlobalDecl GD,
 const TemplateArgumentList *TemplateArgs = nullptr;
 if (GlobalDecl TD = isTemplate(GD, TemplateArgs)) {
   mangleUnscopedTemplateName(TD, AdditionalAbiTags);
-  mangleTemplateArgs(*TemplateArgs);
+  mangleTemplateArgs(asTemplateName(TD), *TemplateArgs);
   return;
 }
 
@@ -952,7 +959,7 @@ void CXXNameMangler::mangleTemplateName(const TemplateDecl 
*TD,
 
   if (DC->isTranslationUnit() || isStdNamespace(DC)) {
 mangleUnscoped

[PATCH] D92854: [flang][driver] Add support for `-fsyntax-only`

[sameeran joshi via Phabricator via cfe-commits]

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

Looks good from my end.
Thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92854

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

[clang] 6326b09 - [AST][RecoveryExpr] Preserve type for broken overrload member call expr.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-12-14T08:50:41+01:00
New Revision: 6326b098852bea51debe415a85eebd1753151cd0

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

LOG: [AST][RecoveryExpr] Preserve type for broken overrload member call expr.

Reviewed By: sammccall

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

Added: 


Modified: 
clang/lib/Sema/SemaOverload.cpp
clang/test/AST/ast-dump-recovery.cpp
clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 5689efe578fa..13d2125d1a28 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -14300,6 +14300,7 @@ ExprResult Sema::BuildCallToMemberFunction(Scope *S, 
Expr *MemExprE,
 UnbridgedCasts.restore();
 
 OverloadCandidateSet::iterator Best;
+bool Succeeded = false;
 switch (CandidateSet.BestViableFunction(*this, UnresExpr->getBeginLoc(),
 Best)) {
 case OR_Success:
@@ -14307,7 +14308,7 @@ ExprResult Sema::BuildCallToMemberFunction(Scope *S, 
Expr *MemExprE,
   FoundDecl = Best->FoundDecl;
   CheckUnresolvedMemberAccess(UnresExpr, Best->FoundDecl);
   if (DiagnoseUseOfDecl(Best->FoundDecl, UnresExpr->getNameLoc()))
-return ExprError();
+break;
   // If FoundDecl is 
diff erent from Method (such as if one is a template
   // and the other a specialization), make sure DiagnoseUseOfDecl is
   // called on both.
@@ -14316,7 +14317,8 @@ ExprResult Sema::BuildCallToMemberFunction(Scope *S, 
Expr *MemExprE,
   // being used.
   if (Method != FoundDecl.getDecl() &&
   DiagnoseUseOfDecl(Method, UnresExpr->getNameLoc()))
-return ExprError();
+break;
+  Succeeded = true;
   break;
 
 case OR_No_Viable_Function:
@@ -14326,27 +14328,25 @@ ExprResult Sema::BuildCallToMemberFunction(Scope *S, 
Expr *MemExprE,
   PDiag(diag::err_ovl_no_viable_member_function_in_call)
   << DeclName << MemExprE->getSourceRange()),
   *this, OCD_AllCandidates, Args);
-  // FIXME: Leaking incoming expressions!
-  return ExprError();
-
+  break;
 case OR_Ambiguous:
   CandidateSet.NoteCandidates(
   PartialDiagnosticAt(UnresExpr->getMemberLoc(),
   PDiag(diag::err_ovl_ambiguous_member_call)
   << DeclName << MemExprE->getSourceRange()),
   *this, OCD_AmbiguousCandidates, Args);
-  // FIXME: Leaking incoming expressions!
-  return ExprError();
-
+  break;
 case OR_Deleted:
   CandidateSet.NoteCandidates(
   PartialDiagnosticAt(UnresExpr->getMemberLoc(),
   PDiag(diag::err_ovl_deleted_member_call)
   << DeclName << MemExprE->getSourceRange()),
   *this, OCD_AllCandidates, Args);
-  // FIXME: Leaking incoming expressions!
-  return ExprError();
+  break;
 }
+// Overload resolution fails, try to recover.
+if (!Succeeded)
+  return BuildRecoveryExpr(chooseRecoveryType(CandidateSet, &Best));
 
 MemExprE = FixOverloadedFunctionReference(MemExprE, FoundDecl, Method);
 

diff  --git a/clang/test/AST/ast-dump-recovery.cpp 
b/clang/test/AST/ast-dump-recovery.cpp
index 2a8346eb0d15..a8da2b8ad449 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -125,6 +125,9 @@ struct Foo2 {
   double func();
   class ForwardClass;
   ForwardClass createFwd();
+
+  int overload();
+  int overload(int, int);
 };
 void test2(Foo2 f) {
   // CHECK:  RecoveryExpr {{.*}} 'double'
@@ -136,6 +139,11 @@ void test2(Foo2 f) {
   // CHECK-NEXT: `-MemberExpr {{.*}} '' .createFwd
   // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'f'
   f.createFwd();
+  // CHECK:  RecoveryExpr {{.*}} 'int' contains-errors
+  // CHECK-NEXT: |-UnresolvedMemberExpr
+  // CHECK-NEXT:`-DeclRefExpr {{.*}} 'Foo2'
+  // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+  f.overload(1);
 }
 
 // CHECK: |-AlignedAttr {{.*}} alignas

diff  --git a/clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp 
b/clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp
index ce43720cb2d3..f12e0083fb0c 100644
--- a/clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp
+++ b/clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp
@@ -9,7 +9,7 @@
 //   parameter types in a base class (rather than conflicting).
 
 template  struct Opaque {};
-template  void expect(Opaque _) {}
+template  void expect(Opaque _) {} // expected-note 4 
{{candidate function template not viable}}
 
 // PR5727
 // This just shouldn't cr

[PATCH] D80109: [AST][RecoveryExpr] Preserve type for broken member call expr.

[Haojian Wu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6326b098852b: [AST][RecoveryExpr] Preserve type for broken 
overrload member call expr. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80109

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp

Index: clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp
===
--- clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp
+++ clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp
@@ -9,7 +9,7 @@
 //   parameter types in a base class (rather than conflicting).
 
 template  struct Opaque {};
-template  void expect(Opaque _) {}
+template  void expect(Opaque _) {} // expected-note 4 {{candidate function template not viable}}
 
 // PR5727
 // This just shouldn't crash.
@@ -134,14 +134,14 @@
   void test() {
 expect<0>(Base().foo());
 expect<1>(Base().foo<0>());
-expect<0>(Derived1().foo()); // expected-error {{no matching member function for call to 'foo'}}
+expect<0>(Derived1().foo()); // expected-error {{no matching member function for call to 'foo'}} expected-error {{no matching function for call to 'expect'}}
 expect<2>(Derived1().foo<0>());
-expect<0>(Derived2().foo()); // expected-error {{no matching member function for call to 'foo'}}
+expect<0>(Derived2().foo()); // expected-error {{no matching member function for call to 'foo'}} expected-error {{no matching function for call to 'expect'}}
 expect<2>(Derived2().foo<0>());
 expect<3>(Derived3().foo());
-expect<1>(Derived3().foo<0>()); // expected-error {{no matching member function for call to 'foo'}}
+expect<1>(Derived3().foo<0>()); // expected-error {{no matching member function for call to 'foo'}} expected-error {{no matching function for call to 'expect'}}
 expect<3>(Derived4().foo());
-expect<1>(Derived4().foo<0>()); // expected-error {{no matching member function for call to 'foo'}}
+expect<1>(Derived4().foo<0>()); // expected-error {{no matching member function for call to 'foo'}} expected-error {{no matching function for call to 'expect'}}
   }
 }
 
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -125,6 +125,9 @@
   double func();
   class ForwardClass;
   ForwardClass createFwd();
+
+  int overload();
+  int overload(int, int);
 };
 void test2(Foo2 f) {
   // CHECK:  RecoveryExpr {{.*}} 'double'
@@ -136,6 +139,11 @@
   // CHECK-NEXT: `-MemberExpr {{.*}} '' .createFwd
   // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'f'
   f.createFwd();
+  // CHECK:  RecoveryExpr {{.*}} 'int' contains-errors
+  // CHECK-NEXT: |-UnresolvedMemberExpr
+  // CHECK-NEXT:`-DeclRefExpr {{.*}} 'Foo2'
+  // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+  f.overload(1);
 }
 
 // CHECK: |-AlignedAttr {{.*}} alignas
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -14300,6 +14300,7 @@
 UnbridgedCasts.restore();
 
 OverloadCandidateSet::iterator Best;
+bool Succeeded = false;
 switch (CandidateSet.BestViableFunction(*this, UnresExpr->getBeginLoc(),
 Best)) {
 case OR_Success:
@@ -14307,7 +14308,7 @@
   FoundDecl = Best->FoundDecl;
   CheckUnresolvedMemberAccess(UnresExpr, Best->FoundDecl);
   if (DiagnoseUseOfDecl(Best->FoundDecl, UnresExpr->getNameLoc()))
-return ExprError();
+break;
   // If FoundDecl is different from Method (such as if one is a template
   // and the other a specialization), make sure DiagnoseUseOfDecl is
   // called on both.
@@ -14316,7 +14317,8 @@
   // being used.
   if (Method != FoundDecl.getDecl() &&
   DiagnoseUseOfDecl(Method, UnresExpr->getNameLoc()))
-return ExprError();
+break;
+  Succeeded = true;
   break;
 
 case OR_No_Viable_Function:
@@ -14326,27 +14328,25 @@
   PDiag(diag::err_ovl_no_viable_member_function_in_call)
   << DeclName << MemExprE->getSourceRange()),
   *this, OCD_AllCandidates, Args);
-  // FIXME: Leaking incoming expressions!
-  return ExprError();
-
+  break;
 case OR_Ambiguous:
   CandidateSet.NoteCandidates(
   PartialDiagnosticAt(UnresExpr->getMemberLoc(),
   PDiag(diag::err_ovl_ambiguous_member_call)
   << DeclName << MemExprE->getSourceRange()),
   *this, OCD_Ambiguous

[PATCH] D92299: [clangd] Go-to-definition on pure virtual method decls jumps to all overrides.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 311501.
hokein marked an inline comment as done.
hokein added a comment.

address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92299

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -349,6 +349,22 @@
   ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range(;
 }
 
+TEST(LocateSymbol, FindOverrides) {
+  auto Code = Annotations(R"cpp(
+class Foo {
+  virtual void $1[[fo^o]]() = 0;
+};
+class Bar : public Foo {
+  void $2[[foo]]() override;
+};
+  )cpp");
+  TestTU TU = TestTU::withCode(Code.code());
+  auto AST = TU.build();
+  EXPECT_THAT(locateSymbolAt(AST, Code.point(), TU.index().get()),
+  UnorderedElementsAre(Sym("foo", Code.range("1"), llvm::None),
+   Sym("foo", Code.range("2"), llvm::None)));
+}
+
 TEST(LocateSymbol, WithIndexPreferredLocation) {
   Annotations SymbolHeader(R"cpp(
 class $p[[Proto]] {};
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -292,6 +292,35 @@
   return D;
 }
 
+std::vector findOverrides(llvm::DenseSet IDs,
+ const SymbolIndex *Index,
+ llvm::StringRef MainFilePath) {
+  if (IDs.empty())
+return {};
+  RelationsRequest Req;
+  Req.Predicate = RelationKind::OverriddenBy;
+  Req.Subjects = std::move(IDs);
+  std::vector Results;
+  Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
+auto DeclLoc =
+indexToLSPLocation(Object.CanonicalDeclaration, MainFilePath);
+if (!DeclLoc) {
+  elog("Find overrides: {0}", DeclLoc.takeError());
+  return;
+}
+Results.emplace_back();
+Results.back().Name = Object.Name.str();
+Results.back().PreferredDeclaration = *DeclLoc;
+auto DefLoc = indexToLSPLocation(Object.Definition, MainFilePath);
+if (!DefLoc) {
+  elog("Failed to convert location: {0}", DefLoc.takeError());
+  return;
+}
+Results.back().Definition = *DefLoc;
+  });
+  return Results;
+}
+
 // Decls are more complicated.
 // The AST contains at least a declaration, maybe a definition.
 // These are up-to-date, and so generally preferred over index results.
@@ -330,10 +359,19 @@
   DeclRelation::TemplatePattern | DeclRelation::Alias;
   auto Candidates =
   getDeclAtPositionWithRelations(AST, CurLoc, Relations, NodeKind);
+  llvm::DenseSet VirtualMethods;
   for (const auto &E : Candidates) {
 const NamedDecl *D = E.first;
-// Special case: void foo() ^override: jump to the overridden method.
 if (const auto *CMD = llvm::dyn_cast(D)) {
+  // Special case: virtual void ^method() = 0: jump to all overrides.
+  // FIXME: extend it to ^virtual, unfortunately, virtual location is not
+  // saved in the AST.
+  if (CMD->isPure()) {
+if (TouchedIdentifier && SM.getSpellingLoc(CMD->getLocation()) ==
+ TouchedIdentifier->location())
+  VirtualMethods.insert(getSymbolID(CMD));
+  }
+  // Special case: void foo() ^override: jump to the overridden method.
   const InheritableAttr *Attr = D->getAttr();
   if (!Attr)
 Attr = D->getAttr();
@@ -420,6 +458,8 @@
 });
   }
 
+  auto Overrides = findOverrides(VirtualMethods, Index, MainFilePath);
+  Result.insert(Result.end(), Overrides.begin(), Overrides.end());
   return Result;
 }
 
@@ -1145,36 +1185,14 @@
  CurLoc.takeError());
 return {};
   }
-  std::vector Results;
   DeclRelationSet Relations =
   DeclRelation::TemplatePattern | DeclRelation::Alias;
-  RelationsRequest Req;
-  Req.Predicate = RelationKind::OverriddenBy;
+  llvm::DenseSet VirtualMethods;
   for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations))
 if (const CXXMethodDecl *CXXMD = llvm::dyn_cast(ND))
   if (CXXMD->isVirtual())
-Req.Subjects.insert(getSymbolID(ND));
-
-  if (Req.Subjects.empty())
-return Results;
-  Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
-auto DeclLoc =
-indexToLSPLocation(Object.CanonicalDeclaration, *MainFilePath);
-if (!DeclLoc) {
-  elog("Find implementation: {0}", DeclLoc.takeError());
-  return;
-}
-LocatedSymbol Loc;
-Loc.Name = Object.Name.str();
-Loc.PreferredDeclaration = *DeclLoc;
-auto DefLoc = indexToLSPLocation(Object.Definition, *MainFilePath);
-if (DefLoc)
-  Loc.Definition = *DefLoc;

[clang-tools-extra] 63ec9e4 - [clangd] Go-to-definition on pure virtual method decls jumps to all overrides.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-12-14T08:56:24+01:00
New Revision: 63ec9e40d10056b0f85605d585e7db0b4146851e

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

LOG: [clangd] Go-to-definition on pure virtual method decls jumps to all 
overrides.

Reviewed By: usaxena95

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

Added: 


Modified: 
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index 8a85507ff14c..ac4543026a9f 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -292,6 +292,35 @@ const NamedDecl *getPreferredDecl(const NamedDecl *D) {
   return D;
 }
 
+std::vector findOverrides(llvm::DenseSet IDs,
+ const SymbolIndex *Index,
+ llvm::StringRef MainFilePath) {
+  if (IDs.empty())
+return {};
+  RelationsRequest Req;
+  Req.Predicate = RelationKind::OverriddenBy;
+  Req.Subjects = std::move(IDs);
+  std::vector Results;
+  Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
+auto DeclLoc =
+indexToLSPLocation(Object.CanonicalDeclaration, MainFilePath);
+if (!DeclLoc) {
+  elog("Find overrides: {0}", DeclLoc.takeError());
+  return;
+}
+Results.emplace_back();
+Results.back().Name = Object.Name.str();
+Results.back().PreferredDeclaration = *DeclLoc;
+auto DefLoc = indexToLSPLocation(Object.Definition, MainFilePath);
+if (!DefLoc) {
+  elog("Failed to convert location: {0}", DefLoc.takeError());
+  return;
+}
+Results.back().Definition = *DefLoc;
+  });
+  return Results;
+}
+
 // Decls are more complicated.
 // The AST contains at least a declaration, maybe a definition.
 // These are up-to-date, and so generally preferred over index results.
@@ -330,10 +359,19 @@ locateASTReferent(SourceLocation CurLoc, const 
syntax::Token *TouchedIdentifier,
   DeclRelation::TemplatePattern | DeclRelation::Alias;
   auto Candidates =
   getDeclAtPositionWithRelations(AST, CurLoc, Relations, NodeKind);
+  llvm::DenseSet VirtualMethods;
   for (const auto &E : Candidates) {
 const NamedDecl *D = E.first;
-// Special case: void foo() ^override: jump to the overridden method.
 if (const auto *CMD = llvm::dyn_cast(D)) {
+  // Special case: virtual void ^method() = 0: jump to all overrides.
+  // FIXME: extend it to ^virtual, unfortunately, virtual location is not
+  // saved in the AST.
+  if (CMD->isPure()) {
+if (TouchedIdentifier && SM.getSpellingLoc(CMD->getLocation()) ==
+ TouchedIdentifier->location())
+  VirtualMethods.insert(getSymbolID(CMD));
+  }
+  // Special case: void foo() ^override: jump to the overridden method.
   const InheritableAttr *Attr = D->getAttr();
   if (!Attr)
 Attr = D->getAttr();
@@ -420,6 +458,8 @@ locateASTReferent(SourceLocation CurLoc, const 
syntax::Token *TouchedIdentifier,
 });
   }
 
+  auto Overrides = findOverrides(VirtualMethods, Index, MainFilePath);
+  Result.insert(Result.end(), Overrides.begin(), Overrides.end());
   return Result;
 }
 
@@ -1145,36 +1185,14 @@ std::vector 
findImplementations(ParsedAST &AST, Position Pos,
  CurLoc.takeError());
 return {};
   }
-  std::vector Results;
   DeclRelationSet Relations =
   DeclRelation::TemplatePattern | DeclRelation::Alias;
-  RelationsRequest Req;
-  Req.Predicate = RelationKind::OverriddenBy;
+  llvm::DenseSet VirtualMethods;
   for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations))
 if (const CXXMethodDecl *CXXMD = llvm::dyn_cast(ND))
   if (CXXMD->isVirtual())
-Req.Subjects.insert(getSymbolID(ND));
-
-  if (Req.Subjects.empty())
-return Results;
-  Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
-auto DeclLoc =
-indexToLSPLocation(Object.CanonicalDeclaration, *MainFilePath);
-if (!DeclLoc) {
-  elog("Find implementation: {0}", DeclLoc.takeError());
-  return;
-}
-LocatedSymbol Loc;
-Loc.Name = Object.Name.str();
-Loc.PreferredDeclaration = *DeclLoc;
-auto DefLoc = indexToLSPLocation(Object.Definition, *MainFilePath);
-if (DefLoc)
-  Loc.Definition = *DefLoc;
-else
-  llvm::consumeError(DefLoc.takeError());
-Results.push_back(Loc);
-  });
-  return Results;
+VirtualMethods.insert(getSymbolID(ND));
+  return findOverrides(std::move(VirtualMethods), Index, *MainFilePath);
 }
 
 ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,

diff  --git a/clang-tools-extra/cla

[PATCH] D92299: [clangd] Go-to-definition on pure virtual method decls jumps to all overrides.

[Haojian Wu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG63ec9e40d100: [clangd] Go-to-definition on pure virtual 
method decls jumps to all overrides. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92299

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -349,6 +349,22 @@
   ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range(;
 }
 
+TEST(LocateSymbol, FindOverrides) {
+  auto Code = Annotations(R"cpp(
+class Foo {
+  virtual void $1[[fo^o]]() = 0;
+};
+class Bar : public Foo {
+  void $2[[foo]]() override;
+};
+  )cpp");
+  TestTU TU = TestTU::withCode(Code.code());
+  auto AST = TU.build();
+  EXPECT_THAT(locateSymbolAt(AST, Code.point(), TU.index().get()),
+  UnorderedElementsAre(Sym("foo", Code.range("1"), llvm::None),
+   Sym("foo", Code.range("2"), llvm::None)));
+}
+
 TEST(LocateSymbol, WithIndexPreferredLocation) {
   Annotations SymbolHeader(R"cpp(
 class $p[[Proto]] {};
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -292,6 +292,35 @@
   return D;
 }
 
+std::vector findOverrides(llvm::DenseSet IDs,
+ const SymbolIndex *Index,
+ llvm::StringRef MainFilePath) {
+  if (IDs.empty())
+return {};
+  RelationsRequest Req;
+  Req.Predicate = RelationKind::OverriddenBy;
+  Req.Subjects = std::move(IDs);
+  std::vector Results;
+  Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
+auto DeclLoc =
+indexToLSPLocation(Object.CanonicalDeclaration, MainFilePath);
+if (!DeclLoc) {
+  elog("Find overrides: {0}", DeclLoc.takeError());
+  return;
+}
+Results.emplace_back();
+Results.back().Name = Object.Name.str();
+Results.back().PreferredDeclaration = *DeclLoc;
+auto DefLoc = indexToLSPLocation(Object.Definition, MainFilePath);
+if (!DefLoc) {
+  elog("Failed to convert location: {0}", DefLoc.takeError());
+  return;
+}
+Results.back().Definition = *DefLoc;
+  });
+  return Results;
+}
+
 // Decls are more complicated.
 // The AST contains at least a declaration, maybe a definition.
 // These are up-to-date, and so generally preferred over index results.
@@ -330,10 +359,19 @@
   DeclRelation::TemplatePattern | DeclRelation::Alias;
   auto Candidates =
   getDeclAtPositionWithRelations(AST, CurLoc, Relations, NodeKind);
+  llvm::DenseSet VirtualMethods;
   for (const auto &E : Candidates) {
 const NamedDecl *D = E.first;
-// Special case: void foo() ^override: jump to the overridden method.
 if (const auto *CMD = llvm::dyn_cast(D)) {
+  // Special case: virtual void ^method() = 0: jump to all overrides.
+  // FIXME: extend it to ^virtual, unfortunately, virtual location is not
+  // saved in the AST.
+  if (CMD->isPure()) {
+if (TouchedIdentifier && SM.getSpellingLoc(CMD->getLocation()) ==
+ TouchedIdentifier->location())
+  VirtualMethods.insert(getSymbolID(CMD));
+  }
+  // Special case: void foo() ^override: jump to the overridden method.
   const InheritableAttr *Attr = D->getAttr();
   if (!Attr)
 Attr = D->getAttr();
@@ -420,6 +458,8 @@
 });
   }
 
+  auto Overrides = findOverrides(VirtualMethods, Index, MainFilePath);
+  Result.insert(Result.end(), Overrides.begin(), Overrides.end());
   return Result;
 }
 
@@ -1145,36 +1185,14 @@
  CurLoc.takeError());
 return {};
   }
-  std::vector Results;
   DeclRelationSet Relations =
   DeclRelation::TemplatePattern | DeclRelation::Alias;
-  RelationsRequest Req;
-  Req.Predicate = RelationKind::OverriddenBy;
+  llvm::DenseSet VirtualMethods;
   for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations))
 if (const CXXMethodDecl *CXXMD = llvm::dyn_cast(ND))
   if (CXXMD->isVirtual())
-Req.Subjects.insert(getSymbolID(ND));
-
-  if (Req.Subjects.empty())
-return Results;
-  Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
-auto DeclLoc =
-indexToLSPLocation(Object.CanonicalDeclaration, *MainFilePath);
-if (!DeclLoc) {
-  elog("Find implementation: {0}", DeclLoc.takeError());
-  return;
-}
-LocatedSymbol Loc;
-Loc.Name = Object.Name.str();
-Loc.PreferredDeclaratio