[PATCH] D126061: [clang] Reject non-declaration C++11 attributes on declarations

[Martin Böhme via Phabricator via cfe-commits]

mboehme marked 2 inline comments as done.
mboehme added a comment.

Thank you @aaron.ballman and @rsmith for the careful and productive reviews!




Comment at: clang/lib/Parse/ParseStmt.cpp:229
   default: {
+bool HaveAttrs = !(CXX11Attrs.empty() && GNUAttrs.empty());
+auto IsStmtAttr = [](ParsedAttr ) { return Attr.isStmtAttr(); };

aaron.ballman wrote:
> I don't insist, but I think it's a tiny bit easier to read this way.
Done!



Comment at: clang/lib/Parse/ParseStmt.cpp:321-323
+for (const ParsedAttr  : CXX11Attrs) {
+  Diag(AL.getRange().getBegin(), diag::warn_attribute_ignored) << AL;
+}

aaron.ballman wrote:
> 
Done!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126061

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

[PATCH] D127685: [clang-format] Never analyze insert/remove braces in the same pass

[Owen Pan 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 rG07b3446d7243: [clang-format] Never analyze insert/remove 
braces in the same pass (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127685

Files:
  clang/lib/Format/Format.cpp


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1833,7 +1833,9 @@
 class BracesInserter : public TokenAnalyzer {
 public:
   BracesInserter(const Environment , const FormatStyle )
-  : TokenAnalyzer(Env, Style) {}
+  : TokenAnalyzer(Env, Style) {
+this->Style.RemoveBracesLLVM = false;
+  }
 
   std::pair
   analyze(TokenAnnotator ,
@@ -1875,7 +1877,9 @@
 class BracesRemover : public TokenAnalyzer {
 public:
   BracesRemover(const Environment , const FormatStyle )
-  : TokenAnalyzer(Env, Style) {}
+  : TokenAnalyzer(Env, Style) {
+this->Style.InsertBraces = false;
+  }
 
   std::pair
   analyze(TokenAnnotator ,


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1833,7 +1833,9 @@
 class BracesInserter : public TokenAnalyzer {
 public:
   BracesInserter(const Environment , const FormatStyle )
-  : TokenAnalyzer(Env, Style) {}
+  : TokenAnalyzer(Env, Style) {
+this->Style.RemoveBracesLLVM = false;
+  }
 
   std::pair
   analyze(TokenAnnotator ,
@@ -1875,7 +1877,9 @@
 class BracesRemover : public TokenAnalyzer {
 public:
   BracesRemover(const Environment , const FormatStyle )
-  : TokenAnalyzer(Env, Style) {}
+  : TokenAnalyzer(Env, Style) {
+this->Style.InsertBraces = false;
+  }
 
   std::pair
   analyze(TokenAnnotator ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 07b3446 - [clang-format] Never analyze insert/remove braces in the same pass

[via cfe-commits]

Author: owenca
Date: 2022-06-14T22:42:54-07:00
New Revision: 07b3446d72436c8372f50c43b45e46efe63182d9

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

LOG: [clang-format] Never analyze insert/remove braces in the same pass

Turn off RemoveBracesLLVM while analyzing InsertBraces and vice
versa to avoid potential interference of each other and better the
performance.

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

Added: 


Modified: 
clang/lib/Format/Format.cpp

Removed: 




diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 65a391dde3d57..df1aa9da16e39 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1833,7 +1833,9 @@ namespace {
 class BracesInserter : public TokenAnalyzer {
 public:
   BracesInserter(const Environment , const FormatStyle )
-  : TokenAnalyzer(Env, Style) {}
+  : TokenAnalyzer(Env, Style) {
+this->Style.RemoveBracesLLVM = false;
+  }
 
   std::pair
   analyze(TokenAnnotator ,
@@ -1875,7 +1877,9 @@ class BracesInserter : public TokenAnalyzer {
 class BracesRemover : public TokenAnalyzer {
 public:
   BracesRemover(const Environment , const FormatStyle )
-  : TokenAnalyzer(Env, Style) {}
+  : TokenAnalyzer(Env, Style) {
+this->Style.InsertBraces = false;
+  }
 
   std::pair
   analyze(TokenAnnotator ,



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

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

[Javier Alvarez via Phabricator via cfe-commits]

Javier-varez updated this revision to Diff 437033.
Javier-varez added a comment.

Rebase on main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

Files:
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/drs/dr21xx.cpp
  clang/test/CXX/special/class.copy/p12-0x.cpp
  clang/test/CXX/special/class.copy/p25-0x.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -12840,7 +12840,7 @@
 https://wg21.link/cwg2171;>2171
 CD4
 Triviality of copy constructor with less-qualified parameter
-Unknown
+Clang 15
   
   
 https://wg21.link/cwg2172;>2172
Index: clang/test/CXX/special/class.copy/p25-0x.cpp
===
--- clang/test/CXX/special/class.copy/p25-0x.cpp
+++ clang/test/CXX/special/class.copy/p25-0x.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
 
 // expected-no-diagnostics
 
@@ -31,7 +32,30 @@
 struct NonConstCopy {
   NonConstCopy =(NonConstCopy &) = default;
 };
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 14
+// Up until (and including) Clang 14, non-const copy assignment operators were not trivial because
+// of dr2171
 using _ = not_trivially_assignable;
+#else
+// In the latest Clang version, all defaulted assignment operators are trivial, even if non-const,
+// because dr2171 is fixed
+static_assert(__has_trivial_assign(NonConstCopy), "");
+static_assert(__is_trivially_assignable(NonConstCopy &, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy &&), "");
+static_assert(__is_trivially_assignable(NonConstCopy &&, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers =(const DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers =(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers =(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_assignable;
+#endif
 
 // class X has no virtual functions
 struct VFn {
Index: clang/test/CXX/special/class.copy/p12-0x.cpp
===
--- clang/test/CXX/special/class.copy/p12-0x.cpp
+++ clang/test/CXX/special/class.copy/p12-0x.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted
+// RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
 
 // expected-no-diagnostics
 
@@ -28,7 +29,25 @@
 struct NonConstCopy {
   NonConstCopy(NonConstCopy &) = default;
 };
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 14
+// Up until (and including) Clang 14, non-const copy constructors were not trivial because of dr2171
 using _ = not_trivially_copyable;
+#else
+// In the latest Clang version, all defaulted constructors are trivial, even if non-const, because
+// dr2171 is fixed.
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, const NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers(const DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_copyable;
+#endif
 
 // class X has no virtual functions
 struct VFn {
Index: clang/test/CXX/drs/dr21xx.cpp
===
--- clang/test/CXX/drs/dr21xx.cpp
+++ clang/test/CXX/drs/dr21xx.cpp
@@ -140,6 +140,33 @@
 #endif
 }
 
+namespace dr2171 { // dr2171: 15
+#if __cplusplus >= 201103L
+
+struct NonConstCopy {
+  NonConstCopy(NonConstCopy &) = default;
+  NonConstCopy =(NonConstCopy &) = default;
+};
+
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy &), "");

[PATCH] D127812: [AArch64] Function multiversioning support added.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:1086
 
+.. option:: -mno-fmv
+

This file is auto-generated. Don't touch it.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1173
 return None;
-  case ARM::BI_BitScanForward:
-  case ARM::BI_BitScanForward64:
+  case clang::ARM::BI_BitScanForward:
+  case clang::ARM::BI_BitScanForward64:

This can be committed separately.



Comment at: clang/test/CodeGen/attr-target-version.c:75
+
+//.
+// CHECK: @__aarch64_cpu_features = external dso_local global { i64 }

Remove `//.` and `// `.

Note: some editors like neovim supports `{` `}` to navigate among paragraphs. 
Adding `//` just adds inconvenience.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127812

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

[PATCH] D125402: [clang][diag] warn if function returns class type by-const-value

[Namgoo Lee via Phabricator via cfe-commits]

nlee updated this revision to Diff 437011.
nlee added a comment.

Warn if:

- It is an assignment expression AND
- RHS is a function call expression AND
- The function returns by-const-value of a class or struct type AND
- The class or struct has non-trivial move assignment operator

Removed `default-ignore` flag and `make check-clang` passes.


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

https://reviews.llvm.org/D125402

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/Misc/warning-wall.c
  clang/test/SemaCXX/warn-pessimizing-return-by-const.cpp

Index: clang/test/SemaCXX/warn-pessimizing-return-by-const.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-pessimizing-return-by-const.cpp
@@ -0,0 +1,293 @@
+// RUN: %clang_cc1 -fsyntax-only -Wpessimizing-return-by-const -std=c++11 -verify %s
+
+// A class with non-trivial move assignment operator
+struct NT1 {
+  char *d;
+  NT1() {
+d = new char[1];
+d[0] = '\0';
+  }
+
+  NT1(const char *c_str) {
+const int to_alloc = length(c_str) + 1;
+d = new char[to_alloc];
+copy(d, c_str);
+  }
+
+  ~NT1() {
+delete[] d;
+  }
+
+  NT1 =(NT1 &) {
+if (this == )
+  return *this;
+delete[] d;
+d = rhs.d;
+rhs.d = new char[1];
+rhs.d[0] = '\0';
+return *this;
+  }
+
+  NT1(const NT1 ) {
+const int to_alloc = length(rhs.d) + 1;
+d = new char[to_alloc];
+copy(d, rhs.d);
+  }
+
+  NT1 =(const NT1 ) {
+if (this == )
+  return *this;
+delete[] d;
+const int to_alloc = length(rhs.d) + 1;
+d = new char[to_alloc];
+copy(d, rhs.d);
+return *this;
+  }
+
+  // c_str : null-terminated c string. If empty, c_str[0] is '\0'. Cannot be nullptr.
+  // Returns the length of the string, excluding the null-terminating byte.
+  // If given an empty string, returns 0.
+  static int length(const char *c_str) {
+int ret = 0;
+while (*c_str++)
+  ++ret;
+return ret;
+  }
+
+  // dst : must be pre-allocated
+  // Copies src to dst, including the null-terminator.
+  static void copy(char *dst, const char *src) {
+while ((*dst++ = *src++))
+  ;
+  }
+
+  bool operator==(const NT1 ) {
+if (length(d) != length(rhs.d))
+  return false;
+int cnt = 0;
+const char *s1 = d;
+const char *s2 = rhs.d;
+while (cnt++ < length(d))
+  if (*s1++ != *s2++)
+return false;
+return true;
+  }
+};
+
+NT1 f_nt1() {
+  return NT1{"f_nt1"};
+}
+
+const NT1 f_nt1_const() { // expected-warning{{const qualifying the return value prevents move semantics}}
+  return NT1{"f_nt1_const"};
+}
+
+void run_nt1() {
+  NT1 nt1;
+  nt1 = f_nt1();
+  nt1 = f_nt1_const(); // expected-note{{copy assignment is invoked here even if move assignment is supported for type 'NT1'}}
+}
+
+// NT2 has non-trivial move assignment operator since the move assignment operator selected
+// to move the base class NT1 is non-trivial
+struct NT2 : public NT1 {
+  NT2() : NT1() {}
+  NT2(const char *c_str) : NT1(c_str) {}
+};
+
+NT2 f_nt2() {
+  return NT2{"f_nt2"};
+}
+
+const NT2 f_nt2_const() { // expected-warning{{const qualifying the return value prevents move semantics}}
+  return NT2{"f_nt2_const"};
+}
+
+void run_nt2() {
+  NT2 nt2;
+  nt2 = f_nt2();
+  nt2 = f_nt2_const(); // expected-note{{copy assignment is invoked here even if move assignment is supported for type 'NT2'}}
+}
+
+// NT3 has non-trivial move assignment operator since the non-static data member of NT1
+// has non-trivial move assignment operator
+struct NT3 {
+  NT1 nt1;
+  NT3() : nt1(NT1{}) {}
+  NT3(const char *c_str) : nt1(NT1(c_str)) {}
+
+  bool operator==(const NT3 ) {
+return nt1 == rhs.nt1;
+  }
+};
+
+NT3 f_nt3() {
+  return NT3{"f_nt3"};
+}
+
+const NT3 f_nt3_const() { // expected-warning{{const qualifying the return value prevents move semantics}}
+  return NT3{"f_nt3_const"};
+}
+
+void run_nt3() {
+  NT3 nt3;
+  nt3 = f_nt3();
+  nt3 = f_nt3_const(); // expected-note{{copy assignment is invoked here even if move assignment is supported for type 'NT3'}}
+}
+
+// NT4 has defaulted move assignment operator which is still non-trivial
+struct NT4 {
+  NT1 nt1;
+  NT4() : nt1(NT1{}) {}
+  NT4(const char *c_str) : nt1(NT1(c_str)) {}
+
+  NT4 =(NT4 &&) = default;
+
+  NT4(const NT4 &) = default;
+  NT4 =(const NT4 &) = default;
+
+  bool operator==(const NT4 ) {
+return nt1 == rhs.nt1;
+  }
+};
+
+NT4 f_nt4() {
+  return NT4{"f_nt4"};
+}
+
+const NT4 f_nt4_const() { // expected-warning{{const qualifying the return value prevents move semantics}}
+  return NT4{"f_nt4_const"};
+}
+
+void run_nt4() {
+  NT4 nt4;
+  nt4 = f_nt4();
+  nt4 = f_nt4_const(); // expected-note{{copy assignment is invoked here even if move assignment is supported for type 'NT4'}}
+}
+
+// 

[clang] 753b915 - [Driver] Improve linking options for target AVR

[Ben Shi via cfe-commits]

Author: Ben Shi
Date: 2022-06-15T02:57:31Z
New Revision: 753b915167bd40375e879866c66be14b7b6a8db8

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

LOG: [Driver] Improve linking options for target AVR

1. Support user specified linker (-fuse-ld)
2. Support user specified linker script (-T)

Reviewed By: MaskRay, haowei

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

Added: 
clang/test/Driver/Inputs/basic_avr_tree/usr/bin/ld.lld

Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Driver/ToolChains/AVR.cpp
clang/test/Driver/avr-toolchain.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 0ef6b0a7e086b..96219f83b0a53 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -36,10 +36,6 @@ def warn_drv_avr_mcu_not_specified : Warning<
   "no target microcontroller specified on command line, cannot "
   "link standard libraries, please pass -mmcu=">,
   InGroup;
-def warn_drv_avr_gcc_not_found: Warning<
-  "no avr-gcc installation can be found on the system, "
-  "cannot link standard libraries">,
-  InGroup;
 def warn_drv_avr_libc_not_found: Warning<
   "no avr-libc installation can be found on the system, "
   "cannot link standard libraries">,

diff  --git a/clang/lib/Driver/ToolChains/AVR.cpp 
b/clang/lib/Driver/ToolChains/AVR.cpp
index 466fc3b20569e..54a9168788585 100644
--- a/clang/lib/Driver/ToolChains/AVR.cpp
+++ b/clang/lib/Driver/ToolChains/AVR.cpp
@@ -375,8 +375,7 @@ AVRToolChain::AVRToolChain(const Driver , const 
llvm::Triple ,
 
   // Only add default libraries if the user hasn't explicitly opted out.
   if (!Args.hasArg(options::OPT_nostdlib) &&
-  !Args.hasArg(options::OPT_nodefaultlibs) &&
-  GCCInstallation.isValid()) {
+  !Args.hasArg(options::OPT_nodefaultlibs) && GCCInstallation.isValid()) {
 GCCInstallPath = GCCInstallation.getInstallPath();
 std::string GCCParentPath(GCCInstallation.getParentLibPath());
 getProgramPaths().push_back(GCCParentPath + "/../bin");
@@ -429,9 +428,14 @@ void AVR::Linker::ConstructJob(Compilation , const 
JobAction ,
   // Compute information about the target AVR.
   std::string CPU = getCPUName(D, Args, getToolChain().getTriple());
   llvm::Optional FamilyName = GetMCUFamilyName(CPU);
+  llvm::Optional AVRLibcRoot = TC.findAVRLibcInstallation();
   llvm::Optional SectionAddressData = GetMCUSectionAddressData(CPU);
 
-  std::string Linker = getToolChain().GetProgramPath(getShortName());
+  // Compute the linker program path, and use GNU "avr-ld" as default.
+  const Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ);
+  std::string Linker = A ? getToolChain().GetLinkerPath(nullptr)
+ : getToolChain().GetProgramPath(getShortName());
+
   ArgStringList CmdArgs;
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
@@ -450,17 +454,11 @@ void AVR::Linker::ConstructJob(Compilation , const 
JobAction ,
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs)) {
 if (!CPU.empty()) {
-Optional FamilyName = GetMCUFamilyName(CPU);
-Optional AVRLibcRoot = TC.findAVRLibcInstallation();
-
   if (!FamilyName) {
 // We do not have an entry for this CPU in the family
 // mapping table yet.
 D.Diag(diag::warn_drv_avr_family_linking_stdlibs_not_implemented)
 << CPU;
-  } else if (TC.getGCCInstallPath().empty()) {
-// We can not link since there is no avr-ld.
-D.Diag(diag::warn_drv_avr_gcc_not_found);
   } else if (!AVRLibcRoot) {
 // No avr-libc found and so no runtime linked.
 D.Diag(diag::warn_drv_avr_libc_not_found);
@@ -473,7 +471,6 @@ void AVR::Linker::ConstructJob(Compilation , const 
JobAction ,
 LinkStdlib = true;
   }
 }
-
 if (!LinkStdlib)
   D.Diag(diag::warn_drv_avr_stdlib_not_linked);
   }
@@ -508,11 +505,15 @@ void AVR::Linker::ConstructJob(Compilation , const 
JobAction ,
 
 CmdArgs.push_back("--end-group");
 
+// Add user specified linker script.
+Args.AddAllArgs(CmdArgs, options::OPT_T);
+
 // Specify the family name as the emulation mode to use.
 // This is almost always required because otherwise avr-ld
 // will assume 'avr2' and warn about the program being larger
 // than the bare minimum supports.
-CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName));
+if (Linker.find("avr-ld") != std::string::npos)
+  CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName));
   }
 
   C.addCommand(std::make_unique(

diff  --git 

[PATCH] D126192: [Driver] Improve linking options for target AVR

[Haowei Wu via Phabricator via cfe-commits]

haowei added a comment.

I tested this patch on our windows builder and it passed as well. 
This patch LGTM. (I cannot click accept as the this review is already closed)


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

https://reviews.llvm.org/D126192

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

[PATCH] D126192: [Driver] Improve linking options for target AVR

[Ben Shi via Phabricator via cfe-commits]

benshi001 added a comment.

In D126192#3583924 , @haowei wrote:

> In D126192#3583915 , @benshi001 
> wrote:
>
>> It seems the file 
>> `/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree/usr/bin/ld.lld`
>>  is missing on your machine, which I have newly created in current patch.
>
> That file was missing. For some reason, it was missing from the patch I 
> downloaded from phabricator( if you look at 
> https://reviews.llvm.org/file/data/2wqpf52swpb3xkcz7lip/PHID-FILE-filks6eto62wxww6m23w/D126192.vson.id436992.diff).
>
> After manually create clang/test/Driver/Inputs/basic_avr_tree/usr/bin/ld.lld 
> and set permission to 0755 (test will fail if I don't do it). All tests will 
> pass under linux-x64. I will need to test it on a Windows host to make sure 
> it all pass, which will take a while.

Thanks. I really appreciate your help.


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

https://reviews.llvm.org/D126192

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

[PATCH] D126192: [Driver] Improve linking options for target AVR

[Haowei Wu via Phabricator via cfe-commits]

haowei added a comment.

In D126192#3583915 , @benshi001 wrote:

> It seems the file 
> `/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree/usr/bin/ld.lld`
>  is missing on your machine, which I have newly created in current patch.

That file was missing. For some reason, it was missing from the patch I 
downloaded from phabricator( if you look at 
https://reviews.llvm.org/file/data/2wqpf52swpb3xkcz7lip/PHID-FILE-filks6eto62wxww6m23w/D126192.vson.id436992.diff).

After manually create clang/test/Driver/Inputs/basic_avr_tree/usr/bin/ld.lld 
and set permission to 0755 (test will fail if I don't do it). All tests will 
pass under linux-x64. I will need to test it on a Windows host to make sure it 
all pass, which will take a while.


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

https://reviews.llvm.org/D126192

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

[PATCH] D127771: [HIP] fix long double size

[Yaxun Liu 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 rGaf9ee3357cec: [HIP] fix long double size (authored by 
yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D127771?vs=436856=436999#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127771

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/CodeGenCUDA/long-double.cu


Index: clang/test/CodeGenCUDA/long-double.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/long-double.cu
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx906 \
+// RUN:   -aux-triple x86_64-unknown-gnu-linux -fcuda-is-device \
+// RUN:   -emit-llvm -o - -x hip %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -triple nvptx \
+// RUN:   -aux-triple x86_64-unknown-gnu-linux -fcuda-is-device \
+// RUN:   -emit-llvm -o - %s 2>&1 | FileCheck %s
+
+// CHECK: @_ZN15infinity_helperIeE5valueE = {{.*}} double 0x47EFD586B834, 
align 8
+// CHECK: @size = {{.*}} i32 8
+
+#include "Inputs/cuda.h"
+
+template  struct infinity_helper {};
+template <> struct infinity_helper { static constexpr long double 
value = 3.4028234e38L; };
+constexpr long double infinity_helper::value;
+__device__ int size = sizeof(long double);
Index: clang/lib/Basic/Targets/AMDGPU.cpp
===
--- clang/lib/Basic/Targets/AMDGPU.cpp
+++ clang/lib/Basic/Targets/AMDGPU.cpp
@@ -462,9 +462,13 @@
   // supported by AMDGPU. Therefore keep its own format for these two types.
   auto SaveLongDoubleFormat = LongDoubleFormat;
   auto SaveFloat128Format = Float128Format;
+  auto SaveLongDoubleWidth = LongDoubleWidth;
+  auto SaveLongDoubleAlign = LongDoubleAlign;
   copyAuxTarget(Aux);
   LongDoubleFormat = SaveLongDoubleFormat;
   Float128Format = SaveFloat128Format;
+  LongDoubleWidth = SaveLongDoubleWidth;
+  LongDoubleAlign = SaveLongDoubleAlign;
   // For certain builtin types support on the host target, claim they are
   // support to pass the compilation of the host code during the device-side
   // compilation.


Index: clang/test/CodeGenCUDA/long-double.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/long-double.cu
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx906 \
+// RUN:   -aux-triple x86_64-unknown-gnu-linux -fcuda-is-device \
+// RUN:   -emit-llvm -o - -x hip %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -triple nvptx \
+// RUN:   -aux-triple x86_64-unknown-gnu-linux -fcuda-is-device \
+// RUN:   -emit-llvm -o - %s 2>&1 | FileCheck %s
+
+// CHECK: @_ZN15infinity_helperIeE5valueE = {{.*}} double 0x47EFD586B834, align 8
+// CHECK: @size = {{.*}} i32 8
+
+#include "Inputs/cuda.h"
+
+template  struct infinity_helper {};
+template <> struct infinity_helper { static constexpr long double value = 3.4028234e38L; };
+constexpr long double infinity_helper::value;
+__device__ int size = sizeof(long double);
Index: clang/lib/Basic/Targets/AMDGPU.cpp
===
--- clang/lib/Basic/Targets/AMDGPU.cpp
+++ clang/lib/Basic/Targets/AMDGPU.cpp
@@ -462,9 +462,13 @@
   // supported by AMDGPU. Therefore keep its own format for these two types.
   auto SaveLongDoubleFormat = LongDoubleFormat;
   auto SaveFloat128Format = Float128Format;
+  auto SaveLongDoubleWidth = LongDoubleWidth;
+  auto SaveLongDoubleAlign = LongDoubleAlign;
   copyAuxTarget(Aux);
   LongDoubleFormat = SaveLongDoubleFormat;
   Float128Format = SaveFloat128Format;
+  LongDoubleWidth = SaveLongDoubleWidth;
+  LongDoubleAlign = SaveLongDoubleAlign;
   // For certain builtin types support on the host target, claim they are
   // support to pass the compilation of the host code during the device-side
   // compilation.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] af9ee33 - [HIP] fix long double size

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2022-06-14T21:57:56-04:00
New Revision: af9ee3357cec931af68a3a0bb83e82de977caa37

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

LOG: [HIP] fix long double size

For amdgpu target long double type is the same as double type.
The width and align of long double type was incorrectly
overridden when copying aux target properties, which
caused assertion in codegen when emitting global
variables with long double type.

This patch fix that by saving and restoring width
and align of long double type.

Reviewed by: Artem Belevich

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

Fixes: SWDEV-335515

Added: 
clang/test/CodeGenCUDA/long-double.cu

Modified: 
clang/lib/Basic/Targets/AMDGPU.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/AMDGPU.cpp 
b/clang/lib/Basic/Targets/AMDGPU.cpp
index a1edc0d53ecba..66e9a22ca1ebb 100644
--- a/clang/lib/Basic/Targets/AMDGPU.cpp
+++ b/clang/lib/Basic/Targets/AMDGPU.cpp
@@ -462,9 +462,13 @@ void AMDGPUTargetInfo::setAuxTarget(const TargetInfo *Aux) 
{
   // supported by AMDGPU. Therefore keep its own format for these two types.
   auto SaveLongDoubleFormat = LongDoubleFormat;
   auto SaveFloat128Format = Float128Format;
+  auto SaveLongDoubleWidth = LongDoubleWidth;
+  auto SaveLongDoubleAlign = LongDoubleAlign;
   copyAuxTarget(Aux);
   LongDoubleFormat = SaveLongDoubleFormat;
   Float128Format = SaveFloat128Format;
+  LongDoubleWidth = SaveLongDoubleWidth;
+  LongDoubleAlign = SaveLongDoubleAlign;
   // For certain builtin types support on the host target, claim they are
   // support to pass the compilation of the host code during the device-side
   // compilation.

diff  --git a/clang/test/CodeGenCUDA/long-double.cu 
b/clang/test/CodeGenCUDA/long-double.cu
new file mode 100644
index 0..454a93ce5f6b6
--- /dev/null
+++ b/clang/test/CodeGenCUDA/long-double.cu
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx906 \
+// RUN:   -aux-triple x86_64-unknown-gnu-linux -fcuda-is-device \
+// RUN:   -emit-llvm -o - -x hip %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -triple nvptx \
+// RUN:   -aux-triple x86_64-unknown-gnu-linux -fcuda-is-device \
+// RUN:   -emit-llvm -o - %s 2>&1 | FileCheck %s
+
+// CHECK: @_ZN15infinity_helperIeE5valueE = {{.*}} double 0x47EFD586B834, 
align 8
+// CHECK: @size = {{.*}} i32 8
+
+#include "Inputs/cuda.h"
+
+template  struct infinity_helper {};
+template <> struct infinity_helper { static constexpr long double 
value = 3.4028234e38L; };
+constexpr long double infinity_helper::value;
+__device__ int size = sizeof(long double);



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

[PATCH] D126192: [Driver] Improve linking options for target AVR

[Ben Shi via Phabricator via cfe-commits]

benshi001 added a comment.

It seems the file 
`/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree/usr/bin/ld.lld`
 is missing on your machine, which I have newly created in current patch.


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

https://reviews.llvm.org/D126192

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

[PATCH] D126192: [Driver] Improve linking options for target AVR

[Ben Shi via Phabricator via cfe-commits]

benshi001 added a comment.

In D126192#3583886 , @haowei wrote:

> Emmm, I applied your latest patch on top of 
> main(6e02e27536b9de25a651cfc9c2966ce471169355 
> ) but I 
> still get a test failure on `Clang :: Driver/avr-toolchain.c`.
>
> `Clang :: Driver/avr-ld.c` no longer fail.
> Error message:
>
>   FAIL: Clang :: Driver/avr-toolchain.c (7349 of 16540)
>    TEST 'Clang :: Driver/avr-toolchain.c' FAILED 
> 
>   Script:
>   --
>   : 'RUN: at line 4';   
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
> --sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
>   
> -resource-dir=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/resource_dir
>  2>&1 | /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
> --check-prefix=CHECK1 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
>   : 'RUN: at line 15';   
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
> --sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
>   _2/opt/local -S 2>&1 | 
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
> --check-prefix=CHECK2 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
>   : 'RUN: at line 21';   
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
> --sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
>   _2 -S 2>&1 | /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
> --check-prefix=CHECK3 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
>   : 'RUN: at line 27';   
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
> 2>&1 | /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
> -check-pre
>   fix=CC1 /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
>   : 'RUN: at line 30';   
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
> -fuse-init-array -fuse-cxa-atexit 2>&1 | 
> /mnt/nvme_sec/SRC/llvm-project/build-re
>   vert-test/bin/FileCheck -check-prefix=CHECK4 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
>   : 'RUN: at line 35';   
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
> --sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
>2>&1 -nostdinc | 
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
> --check-prefix=NOSTDINC 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
>   : 'RUN: at line 36';   
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
> --sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
>2>&1 -nostdlibinc | 
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
> --check-prefix=NOSTDINC 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
>   : 'RUN: at line 39';   
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### --target=avr 
> --sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
>  -mmcu=atmega328 /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver
>   /avr-toolchain.c 2>&1 | 
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
> --check-prefix=NOWARN 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
>   : 'RUN: at line 40';   
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### --target=avr 
> --sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
>  -mmcu=atmega328 -S /mnt/nvme_sec/SRC/llvm-project/clang/test/Dri
>   ver/avr-toolchain.c 2>&1 | 
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
> --check-prefix=NOWARN 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
>   : 'RUN: at line 41';   
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### --target=avr 
> --sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/ 
> -mmcu=atmega328 -S /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolch
>   ain.c 2>&1 | /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
> --check-prefix=NOWARN 
> /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
>   : 'RUN: at line 44';   
> /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang 

[PATCH] D127454: [OpenMP] Initial parsing and sema for 'parallel masked' construct

[Fazlay Rabbi via Phabricator via cfe-commits]

mdfazlay added inline comments.



Comment at: clang/include/clang-c/Index.h:2641
*/
-  CXCursor_TranslationUnit = 300,
+  CXCursor_TranslationUnit = 350,
 

ABataev wrote:
> Does this change affect anything else in the compiler?
We had to add and keep the CXCursor_OMPParallelMaskedDirective with other 
OpenMP directives. I believe it doesn't affect anything else in the compiler. 
We have made necessary changes in "clang/bindings/python/clang/cindex.py" to 
reflect the new value of CXCursor_TranslationUnit.



Comment at: clang/include/clang/AST/StmtOpenMP.h:2315
+///
+class OMPParallelMaskedDirective : public OMPExecutableDirective {
+  friend class ASTStmtReader;

ABataev wrote:
> `final`
Added in the new diff.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127454

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

[PATCH] D127454: [OpenMP] Initial parsing and sema for 'parallel masked' construct

[Fazlay Rabbi via Phabricator via cfe-commits]

mdfazlay updated this revision to Diff 436994.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127454

Files:
  clang/bindings/python/clang/cindex.py
  clang/include/clang-c/Index.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/cfg-openmp.cpp
  clang/test/OpenMP/nesting_of_regions.cpp
  clang/test/OpenMP/parallel_masked_ast_print.cpp
  clang/test/OpenMP/parallel_masked_copyin_messages.cpp
  clang/test/OpenMP/parallel_masked_default_messages.cpp
  clang/test/OpenMP/parallel_masked_firstprivate_messages.cpp
  clang/test/OpenMP/parallel_masked_if_messages.cpp
  clang/test/OpenMP/parallel_masked_message.cpp
  clang/test/OpenMP/parallel_masked_num_threads_messages.cpp
  clang/test/OpenMP/parallel_masked_private_messages.cpp
  clang/test/OpenMP/parallel_masked_proc_bind_messages.cpp
  clang/test/OpenMP/parallel_masked_reduction_messages.cpp
  clang/test/OpenMP/parallel_masked_shared_messages.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -861,6 +861,21 @@
 VersionedClause
   ];
 }
+def OMP_ParallelMasked : Directive<"parallel masked"> {
+  let allowedClauses = [
+VersionedClause,
+VersionedClause,
+VersionedClause,
+VersionedClause,
+VersionedClause,
+VersionedClause,
+VersionedClause,
+VersionedClause,
+VersionedClause,
+VersionedClause,
+VersionedClause
+  ];
+}
 def OMP_ParallelSections : Directive<"parallel sections"> {
   let allowedClauses = [
 VersionedClause,
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -694,6 +694,9 @@
   case Stmt::OMPParallelMasterDirectiveClass:
 K = CXCursor_OMPParallelMasterDirective;
 break;
+  case Stmt::OMPParallelMaskedDirectiveClass:
+K = CXCursor_OMPParallelMaskedDirective;
+break;
   case Stmt::OMPParallelSectionsDirectiveClass:
 K = CXCursor_OMPParallelSectionsDirective;
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2152,6 +2152,7 @@
   void VisitOMPParallelForDirective(const OMPParallelForDirective *D);
   void VisitOMPParallelForSimdDirective(const OMPParallelForSimdDirective *D);
   void VisitOMPParallelMasterDirective(const OMPParallelMasterDirective *D);
+  void VisitOMPParallelMaskedDirective(const OMPParallelMaskedDirective *D);
   void VisitOMPParallelSectionsDirective(const OMPParallelSectionsDirective *D);
   void VisitOMPTaskDirective(const OMPTaskDirective *D);
   void VisitOMPTaskyieldDirective(const OMPTaskyieldDirective *D);
@@ -3073,6 +3074,11 @@
   VisitOMPExecutableDirective(D);
 }
 
+void EnqueueVisitor::VisitOMPParallelMaskedDirective(
+const OMPParallelMaskedDirective *D) {
+  VisitOMPExecutableDirective(D);
+}
+
 void EnqueueVisitor::VisitOMPParallelSectionsDirective(
 const OMPParallelSectionsDirective *D) {
   VisitOMPExecutableDirective(D);
@@ -5766,6 +5772,8 @@
 return cxstring::createRef("OMPParallelForSimdDirective");
   case CXCursor_OMPParallelMasterDirective:
 return cxstring::createRef("OMPParallelMasterDirective");
+  case CXCursor_OMPParallelMaskedDirective:
+return cxstring::createRef("OMPParallelMaskedDirective");
   case CXCursor_OMPParallelSectionsDirective:
 return cxstring::createRef("OMPParallelSectionsDirective");
   case CXCursor_OMPTaskDirective:
Index: clang/test/OpenMP/parallel_masked_shared_messages.cpp
===
--- /dev/null
+++ clang/test/OpenMP/parallel_masked_shared_messages.cpp
@@ -0,0 +1,120 @@
+// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+
+// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+
+void foo() {
+}
+
+bool foobool(int argc) {
+  return argc;
+}
+
+struct S1; // expected-note {{declared here}}
+extern S1 a;
+class S2 {
+  mutable int a;
+
+public:
+  

[PATCH] D126192: [Driver] Improve linking options for target AVR

[Haowei Wu via Phabricator via cfe-commits]

haowei added a comment.

Emmm, I applied your latest patch on top of 
main(6e02e27536b9de25a651cfc9c2966ce471169355 
) but I 
still get a test failure on `Clang :: Driver/avr-toolchain.c`.

`Clang :: Driver/avr-ld.c` no longer fail.
Error message:

  FAIL: Clang :: Driver/avr-toolchain.c (7349 of 16540)
   TEST 'Clang :: Driver/avr-toolchain.c' FAILED 

  Script:
  --
  : 'RUN: at line 4';   
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
--sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
  
-resource-dir=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/resource_dir
 2>&1 | /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
--check-prefix=CHECK1 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
  : 'RUN: at line 15';   
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
--sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
  _2/opt/local -S 2>&1 | 
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
--check-prefix=CHECK2 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
  : 'RUN: at line 21';   
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
--sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
  _2 -S 2>&1 | /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
--check-prefix=CHECK3 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
  : 'RUN: at line 27';   
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
2>&1 | /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck -check-pre
  fix=CC1 /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
  : 'RUN: at line 30';   
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
-fuse-init-array -fuse-cxa-atexit 2>&1 | /mnt/nvme_sec/SRC/llvm-project/build-re
  vert-test/bin/FileCheck -check-prefix=CHECK4 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
  : 'RUN: at line 35';   
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
--sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
   2>&1 -nostdinc | 
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
--check-prefix=NOSTDINC 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
  : 'RUN: at line 36';   
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c --target=avr 
--sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
   2>&1 -nostdlibinc | 
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
--check-prefix=NOSTDINC 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
  : 'RUN: at line 39';   
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### --target=avr 
--sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
 -mmcu=atmega328 /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver
  /avr-toolchain.c 2>&1 | 
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
--check-prefix=NOWARN 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
  : 'RUN: at line 40';   
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### --target=avr 
--sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
 -mmcu=atmega328 -S /mnt/nvme_sec/SRC/llvm-project/clang/test/Dri
  ver/avr-toolchain.c 2>&1 | 
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
--check-prefix=NOWARN 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
  : 'RUN: at line 41';   
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### --target=avr 
--sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/ 
-mmcu=atmega328 -S /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolch
  ain.c 2>&1 | /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 
--check-prefix=NOWARN 
/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchain.c
  : 'RUN: at line 44';   
/mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/clang -### --target=avr 
--sysroot=/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree
 -S /mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/avr-toolchai
  n.c 2>&1 | /mnt/nvme_sec/SRC/llvm-project/build-revert-test/bin/FileCheck 

[PATCH] D127814: [Driver] Force avr-ld to be default linker for target AVR

[Ben Shi via Phabricator via cfe-commits]

benshi001 added a comment.

In D127814#3583854 , @haowei wrote:

> I tested this patch on top of 
> https://github.com/llvm/llvm-project/commit/3b6e166999bb83197c8b70468c2d371c58cf0c25
>  (reverted in 
> https://github.com/llvm/llvm-project/commit/7fae15f9251d3b392058e2fc84898b53619d36ad)
>  and Clang driver tests now pass without issue.

Thanks. You are appreciated to also click `accept` in 
https://reviews.llvm.org/D126192 :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127814

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

[PATCH] D127814: [Driver] Force avr-ld to be default linker for target AVR

[Haowei Wu via Phabricator via cfe-commits]

haowei added a comment.

I tested this patch on top of 
https://github.com/llvm/llvm-project/commit/3b6e166999bb83197c8b70468c2d371c58cf0c25
 (reverted in 
https://github.com/llvm/llvm-project/commit/7fae15f9251d3b392058e2fc84898b53619d36ad)
 and Clang driver tests now pass without issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127814

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

[PATCH] D126192: [Driver] Improve linking options for target AVR

[Ben Shi via Phabricator via cfe-commits]

benshi001 added a comment.

In D126192#3583795 , @haowei wrote:

> I already reverted the change in 
> https://github.com/llvm/llvm-project/commit/7fae15f9251d3b392058e2fc84898b53619d36ad
>  . It is likely the breakage is caused by the fact that Fuchsia's Clang by 
> default use `lld`, see 
> `clang/cmake/caches/Fuchsia-stage2.cmake:set(CLANG_DEFAULT_LINKER lld CACHE 
> STRING "")` . Please let me know if you need to test your new patch with this 
> configuration. I am happy to help.

I have uploaded a new version of my patch, which should fixes the issue on your 
platfrom that lld is default. Could you please help me test ? Please ignore 
another patch.


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

https://reviews.llvm.org/D126192

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

[PATCH] D126192: [Driver] Improve linking options for target AVR

[Ben Shi via Phabricator via cfe-commits]

benshi001 updated this revision to Diff 436992.
benshi001 added a reviewer: haowei.

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

https://reviews.llvm.org/D126192

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Driver/Inputs/basic_avr_tree/usr/bin/ld.lld
  clang/test/Driver/avr-toolchain.c

Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -53,8 +53,22 @@
 // LINKA-NOT: warning: {{.*}} data section address
 
 // RUN: %clang -### --target=avr --sysroot=%S/Inputs/ -mmcu=atmega328 %s 2>&1 | FileCheck --check-prefixes=NOGCC %s
-// NOGCC: warning: no avr-gcc installation can be found on the system, cannot link standard libraries
 // NOGCC: warning: standard library not linked and so no interrupt vector table or compiler runtime routines will be linked
 // NOGCC-NOT: warning: {{.*}} microcontroller
 // NOGCC-NOT: warning: {{.*}} avr-libc
 // NOGCC-NOT: warning: {{.*}} data section address
+
+// RUN: %clang -### --target=avr --sysroot=%S/Inputs/basic_avr_tree -mmcu=atmega328 %s -fuse-ld=avrld 2>&1 | FileCheck --check-prefix=NOLD %s
+// NOLD: error: invalid linker
+
+// RUN: %clang -### --target=avr --sysroot=%S/Inputs/basic_avr_tree -mmcu=atmega328 %s -fuse-ld=%S/Inputs/basic_avr_tree/usr/bin/ld.lld 2>&1 | FileCheck --check-prefix=LLD %s
+// LLD: {{".*lld"}}
+// LLD-NOT: "avr-ld"
+// LLD-NOT: "-mavr5"
+
+// RUN: %clang -### --target=avr --sysroot=%S/Inputs/basic_avr_tree -mmcu=atmega328 %s -T avr.lds 2>&1 | FileCheck --check-prefix=LDS0 %s
+// LDS0: "-T" "avr.lds" "-mavr5"
+
+// RUN: %clang -### --target=avr --sysroot=%S/Inputs/basic_avr_tree -mmcu=atmega328 %s -fuse-ld=%S/Inputs/basic_avr_tree/usr/bin/ld.lld -T avr.lds 2>&1 | FileCheck --check-prefix=LDS1 %s
+// LDS1: "-T" "avr.lds"
+// LDS1-NOT: "-mavr5"
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -375,8 +375,7 @@
 
   // Only add default libraries if the user hasn't explicitly opted out.
   if (!Args.hasArg(options::OPT_nostdlib) &&
-  !Args.hasArg(options::OPT_nodefaultlibs) &&
-  GCCInstallation.isValid()) {
+  !Args.hasArg(options::OPT_nodefaultlibs) && GCCInstallation.isValid()) {
 GCCInstallPath = GCCInstallation.getInstallPath();
 std::string GCCParentPath(GCCInstallation.getParentLibPath());
 getProgramPaths().push_back(GCCParentPath + "/../bin");
@@ -429,9 +428,14 @@
   // Compute information about the target AVR.
   std::string CPU = getCPUName(D, Args, getToolChain().getTriple());
   llvm::Optional FamilyName = GetMCUFamilyName(CPU);
+  llvm::Optional AVRLibcRoot = TC.findAVRLibcInstallation();
   llvm::Optional SectionAddressData = GetMCUSectionAddressData(CPU);
 
-  std::string Linker = getToolChain().GetProgramPath(getShortName());
+  // Compute the linker program path, and use GNU "avr-ld" as default.
+  const Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ);
+  std::string Linker = A ? getToolChain().GetLinkerPath(nullptr)
+ : getToolChain().GetProgramPath(getShortName());
+
   ArgStringList CmdArgs;
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
@@ -450,17 +454,11 @@
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs)) {
 if (!CPU.empty()) {
-Optional FamilyName = GetMCUFamilyName(CPU);
-Optional AVRLibcRoot = TC.findAVRLibcInstallation();
-
   if (!FamilyName) {
 // We do not have an entry for this CPU in the family
 // mapping table yet.
 D.Diag(diag::warn_drv_avr_family_linking_stdlibs_not_implemented)
 << CPU;
-  } else if (TC.getGCCInstallPath().empty()) {
-// We can not link since there is no avr-ld.
-D.Diag(diag::warn_drv_avr_gcc_not_found);
   } else if (!AVRLibcRoot) {
 // No avr-libc found and so no runtime linked.
 D.Diag(diag::warn_drv_avr_libc_not_found);
@@ -473,7 +471,6 @@
 LinkStdlib = true;
   }
 }
-
 if (!LinkStdlib)
   D.Diag(diag::warn_drv_avr_stdlib_not_linked);
   }
@@ -508,11 +505,15 @@
 
 CmdArgs.push_back("--end-group");
 
+// Add user specified linker script.
+Args.AddAllArgs(CmdArgs, options::OPT_T);
+
 // Specify the family name as the emulation mode to use.
 // This is almost always required because otherwise avr-ld
 // will assume 'avr2' and warn about the program being larger
 // than the bare minimum supports.
-CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName));
+if (Linker.find("avr-ld") != std::string::npos)
+  CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName));
   }
 
   C.addCommand(std::make_unique(
Index: 

[PATCH] D126192: [Driver] Improve linking options for target AVR

[Ben Shi via Phabricator via cfe-commits]

benshi001 added a comment.

I think I have fixed in another patch https://reviews.llvm.org/D127814, could 
you please have a look?

@haowei

I have not your environment, my patch works on my ubuntu system, could you 
please try it on your build machine ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126192

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

[PATCH] D127814: [Driver] Force avr-ld to be default linker for target AVR

[Ben Shi via Phabricator via cfe-commits]

benshi001 created this revision.
benshi001 added reviewers: haowei, MaskRay.
Herald added subscribers: StephenFan, Jim, dylanmckay.
Herald added a project: All.
benshi001 requested review of this revision.
Herald added subscribers: cfe-commits, jacquesguan.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127814

Files:
  clang/lib/Driver/ToolChains/AVR.cpp


Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -432,7 +432,9 @@
   llvm::Optional SectionAddressData = GetMCUSectionAddressData(CPU);
 
   // Compute the linker program path, and use GNU "avr-ld" as default.
-  std::string Linker = getToolChain().GetLinkerPath(nullptr);
+  const Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ);
+  std::string Linker = A ? getToolChain().GetLinkerPath(nullptr)
+ : getToolChain().GetProgramPath(getShortName());
 
   ArgStringList CmdArgs;
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);


Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -432,7 +432,9 @@
   llvm::Optional SectionAddressData = GetMCUSectionAddressData(CPU);
 
   // Compute the linker program path, and use GNU "avr-ld" as default.
-  std::string Linker = getToolChain().GetLinkerPath(nullptr);
+  const Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ);
+  std::string Linker = A ? getToolChain().GetLinkerPath(nullptr)
+ : getToolChain().GetProgramPath(getShortName());
 
   ArgStringList CmdArgs;
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126192: [Driver] Improve linking options for target AVR

[Haowei Wu via Phabricator via cfe-commits]

haowei added a comment.

I already reverted the change in 
https://github.com/llvm/llvm-project/commit/7fae15f9251d3b392058e2fc84898b53619d36ad
 . It is likely the breakage is caused by the fact that Fuchsia's Clang by 
default use `lld`, see 
`clang/cmake/caches/Fuchsia-stage2.cmake:set(CLANG_DEFAULT_LINKER lld CACHE 
STRING "")` . Please let me know if you need to test your new patch with this 
configuration. I am happy to help.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126192

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

[PATCH] D127182: [Clang][Modules] Merge availability attributes on imported decls

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added a comment.

In D127182#3582540 , @jansvoboda11 
wrote:

> Should we add a FIXME saying we need to handle other kinds of attributes?

There already is one where we call `mergeInheritableAttributes`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127182

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

[clang] 7fae15f - Revert "[Driver] Improve linking options for target AVR"

[Haowei Wu via cfe-commits]

Author: Haowei Wu
Date: 2022-06-14T17:53:46-07:00
New Revision: 7fae15f9251d3b392058e2fc84898b53619d36ad

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

LOG: Revert "[Driver] Improve linking options for target AVR"

This reverts commit 3b6e166999bb83197c8b70468c2d371c58cf0c25 which
causes Clang Driver test failures on Fuchsia builders.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Driver/ToolChains/AVR.cpp
clang/test/Driver/avr-toolchain.c

Removed: 
clang/test/Driver/Inputs/basic_avr_tree/usr/bin/ld.lld



diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 96219f83b0a5..0ef6b0a7e086 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -36,6 +36,10 @@ def warn_drv_avr_mcu_not_specified : Warning<
   "no target microcontroller specified on command line, cannot "
   "link standard libraries, please pass -mmcu=">,
   InGroup;
+def warn_drv_avr_gcc_not_found: Warning<
+  "no avr-gcc installation can be found on the system, "
+  "cannot link standard libraries">,
+  InGroup;
 def warn_drv_avr_libc_not_found: Warning<
   "no avr-libc installation can be found on the system, "
   "cannot link standard libraries">,

diff  --git a/clang/lib/Driver/ToolChains/AVR.cpp 
b/clang/lib/Driver/ToolChains/AVR.cpp
index 0bdbd5c09a5f..466fc3b20569 100644
--- a/clang/lib/Driver/ToolChains/AVR.cpp
+++ b/clang/lib/Driver/ToolChains/AVR.cpp
@@ -375,7 +375,8 @@ AVRToolChain::AVRToolChain(const Driver , const 
llvm::Triple ,
 
   // Only add default libraries if the user hasn't explicitly opted out.
   if (!Args.hasArg(options::OPT_nostdlib) &&
-  !Args.hasArg(options::OPT_nodefaultlibs) && GCCInstallation.isValid()) {
+  !Args.hasArg(options::OPT_nodefaultlibs) &&
+  GCCInstallation.isValid()) {
 GCCInstallPath = GCCInstallation.getInstallPath();
 std::string GCCParentPath(GCCInstallation.getParentLibPath());
 getProgramPaths().push_back(GCCParentPath + "/../bin");
@@ -428,12 +429,9 @@ void AVR::Linker::ConstructJob(Compilation , const 
JobAction ,
   // Compute information about the target AVR.
   std::string CPU = getCPUName(D, Args, getToolChain().getTriple());
   llvm::Optional FamilyName = GetMCUFamilyName(CPU);
-  llvm::Optional AVRLibcRoot = TC.findAVRLibcInstallation();
   llvm::Optional SectionAddressData = GetMCUSectionAddressData(CPU);
 
-  // Compute the linker program path, and use GNU "avr-ld" as default.
-  std::string Linker = getToolChain().GetLinkerPath(nullptr);
-
+  std::string Linker = getToolChain().GetProgramPath(getShortName());
   ArgStringList CmdArgs;
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
@@ -452,11 +450,17 @@ void AVR::Linker::ConstructJob(Compilation , const 
JobAction ,
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs)) {
 if (!CPU.empty()) {
+Optional FamilyName = GetMCUFamilyName(CPU);
+Optional AVRLibcRoot = TC.findAVRLibcInstallation();
+
   if (!FamilyName) {
 // We do not have an entry for this CPU in the family
 // mapping table yet.
 D.Diag(diag::warn_drv_avr_family_linking_stdlibs_not_implemented)
 << CPU;
+  } else if (TC.getGCCInstallPath().empty()) {
+// We can not link since there is no avr-ld.
+D.Diag(diag::warn_drv_avr_gcc_not_found);
   } else if (!AVRLibcRoot) {
 // No avr-libc found and so no runtime linked.
 D.Diag(diag::warn_drv_avr_libc_not_found);
@@ -469,6 +473,7 @@ void AVR::Linker::ConstructJob(Compilation , const 
JobAction ,
 LinkStdlib = true;
   }
 }
+
 if (!LinkStdlib)
   D.Diag(diag::warn_drv_avr_stdlib_not_linked);
   }
@@ -503,15 +508,11 @@ void AVR::Linker::ConstructJob(Compilation , const 
JobAction ,
 
 CmdArgs.push_back("--end-group");
 
-// Add user specified linker script.
-Args.AddAllArgs(CmdArgs, options::OPT_T);
-
 // Specify the family name as the emulation mode to use.
 // This is almost always required because otherwise avr-ld
 // will assume 'avr2' and warn about the program being larger
 // than the bare minimum supports.
-if (Linker.find("avr-ld") != std::string::npos)
-  CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName));
+CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName));
   }
 
   C.addCommand(std::make_unique(

diff  --git a/clang/test/Driver/Inputs/basic_avr_tree/usr/bin/ld.lld 
b/clang/test/Driver/Inputs/basic_avr_tree/usr/bin/ld.lld
deleted file mode 100755
index e69de29bb2d1..

diff  --git 

[PATCH] D126192: [Driver] Improve linking options for target AVR

[Ben Shi via Phabricator via cfe-commits]

benshi001 added a comment.

I will fix it soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126192

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

[PATCH] D127641: [clang-cl][MSVC] Enable /Zc:alignedNew for C++17 and /Zc:sizedDealloc by default

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 436985.
steplong added a comment.

- Change logic for /Zc:alignedNew and /Zc:sizedDealloc
- Add tests for different /std:c++


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127641

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cl-zc.cpp


Index: clang/test/Driver/cl-zc.cpp
===
--- clang/test/Driver/cl-zc.cpp
+++ clang/test/Driver/cl-zc.cpp
@@ -1,6 +1,17 @@
 // Note: %s must be preceded by --, otherwise it may be interpreted as a
 // command-line option, e.g. on Mac where %s is commonly under /Users.
 
+// RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck 
-check-prefix=SIZED-DEALLOC-DEFAULT %s
+// SIZED-DEALLOC-DEFAULT: "-fsized-deallocation"
+
+// RUN: %clang_cl /c /std:c++11 -### -- %s 2>&1 | FileCheck 
-check-prefix=ALIGNED-NEW-BEFORE-CPP17-DEFAULT %s
+// RUN: %clang_cl /c /std:c++14 -### -- %s 2>&1 | FileCheck 
-check-prefix=ALIGNED-NEW-BEFORE-CPP17-DEFAULT %s
+// RUN: %clang_cl /c /std:c++17 -### -- %s 2>&1 | FileCheck 
-check-prefix=ALIGNED-NEW-CPP17ONWARDS-DEFAULT %s
+// RUN: %clang_cl /c /std:c++20 -### -- %s 2>&1 | FileCheck 
-check-prefix=ALIGNED-NEW-CPP17ONWARDS-DEFAULT %s
+// RUN: %clang_cl /c /std:c++latest -### -- %s 2>&1 | FileCheck 
-check-prefix=ALIGNED-NEW-CPP17ONWARDS-DEFAULT %s
+// ALIGNED-NEW-BEFORE-CPP17-DEFAULT-NOT: "-faligned-allocation"
+// ALIGNED-NEW-CPP17ONWARDS-DEFAULT: "-faligned-allocation"
+
 // RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck 
-check-prefix=TRIGRAPHS-DEFAULT %s
 // cc1 will disable trigraphs for -fms-compatibility as long as -ftrigraphs
 // isn't explicitly passed.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6483,6 +6483,7 @@
 }
 CmdArgs.push_back(LanguageStandard.data());
   }
+  bool IsCPP17Onwards = false;
   if (ImplyVCPPCXXVer) {
 StringRef LanguageStandard;
 if (const Arg *StdArg = Args.getLastArg(options::OPT__SLASH_std)) {
@@ -6506,6 +6507,9 @@
 }
 
 CmdArgs.push_back(LanguageStandard.data());
+
+IsCPP17Onwards =
+LanguageStandard != "-std=c++11" && LanguageStandard != "-std=c++14";
   }
 
   Args.addOptInFlag(CmdArgs, options::OPT_fborland_extensions,
@@ -6635,9 +6639,14 @@
 options::OPT_fno_relaxed_template_template_args);
 
   // -fsized-deallocation is off by default, as it is an ABI-breaking change 
for
-  // most platforms.
-  Args.addOptInFlag(CmdArgs, options::OPT_fsized_deallocation,
-options::OPT_fno_sized_deallocation);
+  // most platforms. MSVC turns on /Zc:sizedDealloc by default.
+  if (IsWindowsMSVC &&
+  !Args.getLastArg(options::OPT_fsized_deallocation,
+   options::OPT_fno_sized_deallocation))
+CmdArgs.push_back("-fsized-deallocation");
+  else
+Args.addOptInFlag(CmdArgs, options::OPT_fsized_deallocation,
+  options::OPT_fno_sized_deallocation);
 
   // -faligned-allocation is on by default in C++17 onwards and otherwise off
   // by default.
@@ -6648,6 +6657,8 @@
   CmdArgs.push_back("-fno-aligned-allocation");
 else
   CmdArgs.push_back("-faligned-allocation");
+  } else if (IsCPP17Onwards) {
+CmdArgs.push_back("-faligned-allocation");
   }
 
   // The default new alignment can be specified using a dedicated option or via


Index: clang/test/Driver/cl-zc.cpp
===
--- clang/test/Driver/cl-zc.cpp
+++ clang/test/Driver/cl-zc.cpp
@@ -1,6 +1,17 @@
 // Note: %s must be preceded by --, otherwise it may be interpreted as a
 // command-line option, e.g. on Mac where %s is commonly under /Users.
 
+// RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck -check-prefix=SIZED-DEALLOC-DEFAULT %s
+// SIZED-DEALLOC-DEFAULT: "-fsized-deallocation"
+
+// RUN: %clang_cl /c /std:c++11 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-BEFORE-CPP17-DEFAULT %s
+// RUN: %clang_cl /c /std:c++14 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-BEFORE-CPP17-DEFAULT %s
+// RUN: %clang_cl /c /std:c++17 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-CPP17ONWARDS-DEFAULT %s
+// RUN: %clang_cl /c /std:c++20 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-CPP17ONWARDS-DEFAULT %s
+// RUN: %clang_cl /c /std:c++latest -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-CPP17ONWARDS-DEFAULT %s
+// ALIGNED-NEW-BEFORE-CPP17-DEFAULT-NOT: "-faligned-allocation"
+// ALIGNED-NEW-CPP17ONWARDS-DEFAULT: "-faligned-allocation"
+
 // RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck -check-prefix=TRIGRAPHS-DEFAULT %s
 // cc1 will disable trigraphs for -fms-compatibility as long as -ftrigraphs
 // isn't explicitly passed.
Index: clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D127807: [clang-tidy] Properly forward clang-tidy output when running tests

[Nicolas van Kempen via Phabricator via cfe-commits]

nicovank created this revision.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
nicovank retitled this revision from "[clang-tidy] Remove encode when 
outputting tool ouput running tests" to "[clang-tidy] Properly forward 
clang-tidy output when running tests".
nicovank updated this revision to Diff 436977.
nicovank added a comment.
nicovank published this revision for review.
nicovank added reviewers: rnk, alexfh.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Rebase.


When running tests, the check_clang_tidy script encodes the output string, 
making it hard to read when debugging checks. This removes the `.encode()` call.

Test Plan:
Making a new default check for testing (as of right now, it includes a failing 
test):

  [~/llvm-project/clang-tools-extra] python3 clang-tidy/add_new_check.py 
bugprone example
  <...>

Pre-changes:

  [~/llvm-project/build] ninja check-clang-tools
  <...>
   clang-tidy output ---
  b"1 warning 
generated.\n/data/users/nvankempen/llvm-project/build/Debug/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-example.cpp.tmp.cpp:4:6:
 warning: function 'f' is insufficiently awesome [bugprone-example]\nvoid 
f();\n 
^\n/data/users/nvankempen/llvm-project/build/Debug/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-example.cpp.tmp.cpp:4:6:
 note: insert 'awesome'\nvoid f();\n ^\n awesome_\n"
  
  --
  <...>

Post-changes:

  [~/llvm-project/build] ninja check-clang-tools
  <...>
   clang-tidy output ---
  1 warning generated.
  
/data/users/nvankempen/llvm-project/build/Debug/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-example.cpp.tmp.cpp:4:6:
 warning: function 'f' is insufficiently awesome [bugprone-example]
  void f();
   ^
  
/data/users/nvankempen/llvm-project/build/Debug/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-example.cpp.tmp.cpp:4:6:
 note: insert 'awesome'
  void f();
   ^
   awesome_
  
  --
  <...>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127807

Files:
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py


Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -173,13 +173,13 @@
 print('Running ' + repr(args) + '...')
 clang_tidy_output = try_run(args)
 print(' clang-tidy output ---')
-print(clang_tidy_output.encode())
-
print('\n--')
+print(clang_tidy_output)
+print('--')
 
 diff_output = try_run(['diff', '-u', self.original_file_name, 
self.temp_file_name], False)
-print('-- Fixes 
-\n' +
-  diff_output +
-  
'\n--')
+print('-- Fixes -')
+print(diff_output)
+print('--')
 return clang_tidy_output
 
   def check_fixes(self):


Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -173,13 +173,13 @@
 print('Running ' + repr(args) + '...')
 clang_tidy_output = try_run(args)
 print(' clang-tidy output ---')
-print(clang_tidy_output.encode())
-print('\n--')
+print(clang_tidy_output)
+print('--')
 
 diff_output = try_run(['diff', '-u', self.original_file_name, self.temp_file_name], False)
-print('-- Fixes -\n' +
-  diff_output +
-  '\n--')
+print('-- Fixes -')
+print(diff_output)
+print('--')
 return clang_tidy_output
 
   def check_fixes(self):
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127163: [clang] Add -fsanitize=memtag-globals (no-op).

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis accepted this revision.
eugenis 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/D127163/new/

https://reviews.llvm.org/D127163

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

[PATCH] D127745: [clang][dataflow] Rename `getPointeeLoc` to `getReferentLoc` for ReferenceValue.

[Dmitri Gribenko 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 rG97d69cdaf324: [clang][dataflow] Rename `getPointeeLoc` to 
`getReferentLoc` for ReferenceValue. (authored by wyt, committed by gribozavr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127745

Files:
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -293,29 +293,29 @@
   // [[p]]
 }
   )";
-  runDataflow(Code,
-  [](llvm::ArrayRef<
- std::pair>>
- Results,
- ASTContext ) {
-ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
-const Environment  = Results[0].second.Env;
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
 
-const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-ASSERT_THAT(FooDecl, NotNull());
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
 
-const StorageLocation *FooLoc =
-Env.getStorageLocation(*FooDecl, SkipPast::None);
-ASSERT_TRUE(isa_and_nonnull(FooLoc));
+const StorageLocation *FooLoc =
+Env.getStorageLocation(*FooDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
-const ReferenceValue *FooVal =
-cast(Env.getValue(*FooLoc));
-const StorageLocation  = FooVal->getPointeeLoc();
-EXPECT_TRUE(isa());
+const ReferenceValue *FooVal =
+cast(Env.getValue(*FooLoc));
+const StorageLocation  = FooVal->getReferentLoc();
+EXPECT_TRUE(isa());
 
-const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
-EXPECT_TRUE(isa_and_nonnull(FooPointeeVal));
-  });
+const Value *FooReferentVal = Env.getValue(FooReferentLoc);
+EXPECT_TRUE(isa_and_nonnull(FooReferentVal));
+  });
 }
 
 TEST_F(TransferTest, SelfReferentialReferenceVarDecl) {
@@ -397,31 +397,31 @@
 const auto *FooLoc = cast(
 Env.getStorageLocation(*FooDecl, SkipPast::None));
 const auto *FooVal = cast(Env.getValue(*FooLoc));
-const auto *FooPointeeVal =
-cast(Env.getValue(FooVal->getPointeeLoc()));
+const auto *FooReferentVal =
+cast(Env.getValue(FooVal->getReferentLoc()));
 
 const auto *BarVal =
-cast(FooPointeeVal->getChild(*BarDecl));
-const auto *BarPointeeVal =
-cast(Env.getValue(BarVal->getPointeeLoc()));
+cast(FooReferentVal->getChild(*BarDecl));
+const auto *BarReferentVal =
+cast(Env.getValue(BarVal->getReferentLoc()));
 
 const auto *FooRefVal =
-cast(BarPointeeVal->getChild(*FooRefDecl));
-const StorageLocation  = FooRefVal->getPointeeLoc();
-EXPECT_THAT(Env.getValue(FooRefPointeeLoc), IsNull());
+cast(BarReferentVal->getChild(*FooRefDecl));
+const StorageLocation  = FooRefVal->getReferentLoc();
+EXPECT_THAT(Env.getValue(FooReferentLoc), IsNull());
 
 const auto *FooPtrVal =
-cast(BarPointeeVal->getChild(*FooPtrDecl));
+cast(BarReferentVal->getChild(*FooPtrDecl));
 const StorageLocation  = FooPtrVal->getPointeeLoc();
 EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
 
 const auto *BazRefVal =
-cast(BarPointeeVal->getChild(*BazRefDecl));
-const StorageLocation  = BazRefVal->getPointeeLoc();
-EXPECT_THAT(Env.getValue(BazRefPointeeLoc), NotNull());
+cast(BarReferentVal->getChild(*BazRefDecl));
+const StorageLocation  = BazRefVal->getReferentLoc();
+EXPECT_THAT(Env.getValue(BazReferentLoc), NotNull());
 
 const auto *BazPtrVal =
-cast(BarPointeeVal->getChild(*BazPtrDecl));
+cast(BarReferentVal->getChild(*BazPtrDecl));
 const StorageLocation  = BazPtrVal->getPointeeLoc();
 EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
   });
@@ -564,8 +564,8 @@
 
 const auto *FooRefVal =
 cast(BarPointeeVal->getChild(*FooRefDecl));
-const StorageLocation  = FooRefVal->getPointeeLoc();
-EXPECT_THAT(Env.getValue(FooRefPointeeLoc), IsNull());
+const StorageLocation  = 

[clang] 97d69cd - [clang][dataflow] Rename `getPointeeLoc` to `getReferentLoc` for ReferenceValue.

[Dmitri Gribenko via cfe-commits]

Author: Wei Yi Tee
Date: 2022-06-15T00:53:30+02:00
New Revision: 97d69cdaf324ec520b3f7685ef05f8e207a7bd3b

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

LOG: [clang][dataflow] Rename `getPointeeLoc` to `getReferentLoc` for 
ReferenceValue.

We distinguish between the referent location for `ReferenceValue` and pointee 
location for `PointerValue`. The former must be non-empty but the latter may be 
empty in the case of a `nullptr`

Reviewed By: gribozavr2, sgatev

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/Value.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/Value.h 
b/clang/include/clang/Analysis/FlowSensitive/Value.h
index 3ac4b8c60996b..9c93456346b23 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Value.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -170,17 +170,17 @@ class IntegerValue : public Value {
 /// in C.
 class ReferenceValue final : public Value {
 public:
-  explicit ReferenceValue(StorageLocation )
-  : Value(Kind::Reference), PointeeLoc(PointeeLoc) {}
+  explicit ReferenceValue(StorageLocation )
+  : Value(Kind::Reference), ReferentLoc(ReferentLoc) {}
 
   static bool classof(const Value *Val) {
 return Val->getKind() == Kind::Reference;
   }
 
-  StorageLocation () const { return PointeeLoc; }
+  StorageLocation () const { return ReferentLoc; }
 
 private:
-  StorageLocation 
+  StorageLocation 
 };
 
 /// Models a symbolic pointer. Specifically, any value of type `T*`.

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 033ef6afbeb2f..370b420b80755 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -53,7 +53,7 @@ llvm::DenseMap intersectDenseMaps(const 
llvm::DenseMap ,
 static bool areEquivalentIndirectionValues(Value *Val1, Value *Val2) {
   if (auto *IndVal1 = dyn_cast(Val1)) {
 auto *IndVal2 = cast(Val2);
-return >getPointeeLoc() == >getPointeeLoc();
+return >getReferentLoc() == >getReferentLoc();
   }
   if (auto *IndVal1 = dyn_cast(Val1)) {
 auto *IndVal2 = cast(Val2);
@@ -522,7 +522,7 @@ StorageLocation ::skip(StorageLocation , 
SkipPast SP) const {
 // References cannot be chained so we only need to skip past one level of
 // indirection.
 if (auto *Val = dyn_cast_or_null(getValue(Loc)))
-  return Val->getPointeeLoc();
+  return Val->getReferentLoc();
 return Loc;
   case SkipPast::ReferenceThenPointer:
 StorageLocation  = skip(Loc, SkipPast::Reference);

diff  --git 
a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp 
b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index f837fe31c6cd8..23a9d964b5b15 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -236,7 +236,7 @@ StorageLocation 
*maybeInitializeOptionalValueMember(QualType Q,
   // `Value` representing the optional (here, `OptionalVal`).
   if (auto *ValueProp = OptionalVal.getProperty("value")) {
 auto *ValueRef = clang::cast(ValueProp);
-auto  = ValueRef->getPointeeLoc();
+auto  = ValueRef->getReferentLoc();
 if (Env.getValue(ValueLoc) == nullptr) {
   // The property was previously set, but the value has been lost. This can
   // happen, for example, because of an environment merge (where the two

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 5780968d81591..e3c97367b17ca 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -293,29 +293,29 @@ TEST_F(TransferTest, ReferenceVarDecl) {
   // [[p]]
 }
   )";
-  runDataflow(Code,
-  [](llvm::ArrayRef<
- std::pair>>
- Results,
- ASTContext ) {
-ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
-const Environment  = Results[0].second.Env;
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
 
-const ValueDecl *FooDecl = 

[PATCH] D126984: [clang] Add support for optimize function attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

FWIW, I think we should have these attributes as spelled here, just w/ proper 
pass manager integration which then requires an RFC.
That said, I'm not opposed to this as an incremental step, albeit confusing 
until the PM support is integrated if we allow O1 
/2/3/fast


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

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

[PATCH] D127803: Generate the capture for field when the field is used with implicit default.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 created this revision.
jyu2 added reviewers: ABataev, jdoerfert, mikerice.
jyu2 added projects: OpenMP, clang.
Herald added a project: All.
jyu2 requested review of this revision.
Herald added a subscriber: sstefan1.

This is to fix assert when field is referenced and default clause is used.
The problem of assert is the capture is not generated for the field.
This patch is to generate capture when the field is used with implicit default, 
use it in the code, and save the capture off to make sure it is considered from 
that point and add first/private clauses.
1>Add new sema function isOpenMPFDCaptureDecl, the isOpenMPCaptureDecl is 
called before return.  The capture reference is

  generated and saved in DSA.  It is called in BuildFieldReferenceExpr.

2>Add new function insertDSA to save newly generated Capture info for FD.
3>Add addition argument in hasDSA to check default attribute for 
default(first|private).
4>Add new field ImplicitFD in DSAVarData used in VisitDeclRefExpr and when 
building the clause.
5>Change isOpenMPPrivateDecl where stop propagate the capture from the 
enclosing region for private variable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127803

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/OpenMP/default_firstprivate_ast_print.cpp
  clang/test/OpenMP/default_private_ast_print.cpp

Index: clang/test/OpenMP/default_private_ast_print.cpp
===
--- clang/test/OpenMP/default_private_ast_print.cpp
+++ clang/test/OpenMP/default_private_ast_print.cpp
@@ -96,4 +96,50 @@
   // DUMP-NEXT:  -DeclRefExpr {{.*}} 'a'
   // DUMP-NEXT:  -DeclRefExpr {{.*}} 'yy'
 }
+
+void zoo(int);
+struct A {
+  int z;
+  int f;
+  A(); ~A();
+  void foo() {
+   #pragma omp parallel private(z) default(private)
+   {
+ z++; f++; zoo(z+f); f++;
+   }
+  }
+// PRINT:#pragma omp parallel private(this->z) default(private)
+// DUMP: -OMPParallelDirective
+// DUMP-NEXT:  -OMPPrivateClause
+// DUMP-NEXT:-DeclRefExpr {{.*}} 'z'
+// DUMP-NEXT:  -OMPDefaultClause
+// DUMP-NEXT:  -OMPPrivateClause
+// DUMP-NEXT:-DeclRefExpr {{.*}} 'f'
+// DUMP: -CXXThisExpr {{.*}} 'A *' implicit this
+  void bar() {
+#pragma omp parallel private(z) default(private)
+{
+  #pragma omp parallel private(z) default(private)
+  {
+z++; f++; zoo(z+f); f++;
+  }
+}
+  }
+// PRINT:#pragma omp parallel private(this->z) default(private)
+// PRINT:  #pragma omp parallel private(this->z) default(private)
+// DUMP: -OMPParallelDirective
+// DUMP-NEXT:  -OMPPrivateClause
+// DUMP-NEXT:-DeclRefExpr {{.*}} 'z'
+// DUMP-NEXT:  -OMPDefaultClause
+// DUMP:   -OMPParallelDirective
+// DUMP-NEXT:-OMPPrivateClause
+// DUMP-NEXT:   -DeclRefExpr {{.*}} 'z'
+// DUMP-NEXT:-OMPDefaultClause
+// DUMP-NEXT:-OMPPrivateClause {{.*}} 
+// DUMP-NEXT:   -DeclRefExpr {{.*}} 'f'
+// DUMP: -CXXThisExpr
+// DUMP: -MemberExpr
+// DUMP-NEXT:   -CXXThisExpr
+// DUMP: -CXXThisExpr
+};
 #endif // HEADER
Index: clang/test/OpenMP/default_firstprivate_ast_print.cpp
===
--- clang/test/OpenMP/default_firstprivate_ast_print.cpp
+++ clang/test/OpenMP/default_firstprivate_ast_print.cpp
@@ -99,4 +99,53 @@
   // DUMP-NEXT: -DeclRefExpr {{.*}} 'yy'
   // DUMP-NEXT: -DeclRefExpr {{.*}} 'y'
 }
+void zoo(int);
+struct A {
+  int z;
+  int f;
+  A(); ~A();
+  void foo() {
+   #pragma omp parallel firstprivate(z) default(firstprivate)
+   {
+ z++; f++; zoo(z+f); f++;
+   }
+  }
+  // PRINT:  #pragma omp parallel firstprivate(this->z) default(firstprivate)
+  // DUMP:   -OMPParallelDirective
+  // DUMP-NEXT: -OMPFirstprivateClause
+  // DUMP-NEXT: -DeclRefExpr {{.*}} 'z'
+  // DUMP-NEXT: -OMPDefaultClause
+  // DUMP-NEXT: -OMPFirstprivateClause {{.*}} 
+  // DUMP-NEXT: -DeclRefExpr {{.*}} 'f'
+  // DUMP:  -CXXThisExpr {{.*}} 'A *' implicit this
+  // DUMP-NEXT: -DeclRefExpr {{.*}} 'z'
+  // DUMP-NEXT: -DeclRefExpr {{.*}} 'f'
+  void bar() {
+#pragma omp parallel firstprivate(z) default(firstprivate)
+{
+  #pragma omp parallel private(z) default(firstprivate)
+  {
+z++; f++; zoo(z+f); f++;
+  }
+}
+  }
+  // PRINT:  #pragma omp parallel firstprivate(this->z) default(firstprivate)
+  // PRINT:#pragma omp parallel private(this->z) default(firstprivate)
+  // DUMP: -OMPParallelDirective
+  // DUMP-NEXT: -OMPFirstprivateClause
+  // DUMP-NEXT:  -DeclRefExpr {{.*}} 'z'
+  // DUMP-NEXT:  -OMPDefaultClause
+  // DUMP:-OMPParallelDirective
+  // DUMP-NEXT:-OMPPrivateClaus
+  // DUMP-NEXT: -DeclRefExpr {{.*}} 'z'
+  // DUMP-NEXT: -OMPDefaultClause
+  // DUMP-NEXT: -OMPFirstprivateClause {{.*}} 
+  // DUMP-NEXT:  

[PATCH] D127802: [HLSL] Support HLSL vector initializers

[Xiang Li via Phabricator via cfe-commits]

python3kgae added inline comments.



Comment at: clang/test/SemaHLSL/BuiltIns/vector-constructors-erros.hlsl:8
+void entry() {
+  float2 LilVec = float2(1.0, 2.0);
+  float2 BrokenVec = float2(1.0, 2.0, 3.0); // expected-error{{excess elements 
in vector initializer}}

Could we have test case for array/struct when initialize vector?
Like
float a[2] = {1.2, 3.2};
float4 arrayVec = float4(a, 1.0, 2.0);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127802

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

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Conflict markers fixed by 6ccc2733e72017999a94c10147a71ff595286080 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

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

[PATCH] D125723: [MSVC] Add support for MSVC pragma optimize

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 436954.
steplong edited the summary of this revision.
steplong added a comment.

- Change logic to only handle empty optimization list
- Fixup doc and tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125723

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/pragma-msvc-optimize.c
  clang/test/Preprocessor/pragma_microsoft.c

Index: clang/test/Preprocessor/pragma_microsoft.c
===
--- clang/test/Preprocessor/pragma_microsoft.c
+++ clang/test/Preprocessor/pragma_microsoft.c
@@ -228,7 +228,13 @@
 #pragma optimize("g"  // expected-warning{{expected ',' in '#pragma optimize'}}
 #pragma optimize("g", // expected-warning{{missing argument to '#pragma optimize'; expected 'on' or 'off'}}
 #pragma optimize("g",xyz  // expected-warning{{unexpected argument 'xyz' to '#pragma optimize'; expected 'on' or 'off'}}
-#pragma optimize("g",on)  // expected-warning{{#pragma optimize' is not supported}}
+#pragma optimize("g",on)  // expected-warning{{unexpected argument 'g' to '#pragma optimize'; expected ""}}
+#pragma optimize("",on)  // no-warning
+#pragma optimize("", on) asdf // expected-warning{{extra tokens at end of '#pragma optimize'}}
+
+void pragma_optimize_foo() {
+#pragma optimize("", on) // expected-error {{'#pragma optimize' can only appear at file scope}}
+}
 
 #pragma execution_character_set // expected-warning {{expected '('}}
 #pragma execution_character_set(// expected-warning {{expected 'push' or 'pop'}}
Index: clang/test/CodeGen/pragma-msvc-optimize.c
===
--- /dev/null
+++ clang/test/CodeGen/pragma-msvc-optimize.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -O2 -emit-llvm -fms-extensions -o - %s | FileCheck %s
+
+#pragma optimize("", off)
+
+// CHECK: define{{.*}} void @f0(){{.*}} #[[OPTNONE:[0-9]+]]
+void f0() {}
+
+// CHECK: define{{.*}} void @f1(){{.*}} #[[OPTNONE]]
+void f1() {}
+
+#pragma optimize("", on)
+
+// CHECK: define{{.*}} void @f2(){{.*}} #[[NO_OPTNONE:[0-9]+]]
+void f2() {}
+
+// CHECK: define{{.*}} void @f3(){{.*}} #[[NO_OPTNONE]]
+void f3() {}
+
+// CHECK: attributes #[[OPTNONE]] = {{{.*}}optnone{{.*}}}
+// CHECK-NOT: attributes #[[NO_OPTNONE]] = {{{.*}}optnone{{.*}}}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10192,6 +10192,7 @@
 AddRangeBasedOptnone(NewFD);
 AddImplicitMSFunctionNoBuiltinAttr(NewFD);
 AddSectionMSAllocText(NewFD);
+ModifyFnAttributesMSPragmaOptimize(NewFD);
   }
 
   // If this is the first declaration of an extern C variable, update
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -1096,6 +1096,15 @@
 OptimizeOffPragmaLocation = PragmaLoc;
 }
 
+void Sema::ActOnPragmaMSOptimize(SourceLocation Loc, bool IsOn) {
+  if (!CurContext->getRedeclContext()->isFileContext()) {
+Diag(Loc, diag::err_pragma_expected_file_scope) << "optimize";
+return;
+  }
+
+  MSPragmaOptimizeIsOn = IsOn;
+}
+
 void Sema::ActOnPragmaMSFunction(
 SourceLocation Loc, const llvm::SmallVectorImpl ) {
   if (!CurContext->getRedeclContext()->isFileContext()) {
@@ -1129,6 +1138,17 @@
   }
 }
 
+void Sema::ModifyFnAttributesMSPragmaOptimize(FunctionDecl *FD) {
+  assert(MSPragmaOptimizeValidParams.empty());
+
+  // Don't modify the function attributes if it's "on". "on" resets the
+  // optimizations to the ones listed on the command line
+  if (!MSPragmaOptimizeIsOn) {
+AddOptnoneAttributeIfNoConflicts(FD, FD->getBeginLoc());
+return;
+  }
+}
+
 void Sema::AddOptnoneAttributeIfNoConflicts(FunctionDecl *FD,
 SourceLocation Loc) {
   // Don't add a conflicting attribute. No diagnostic is needed.
Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -255,12 +255,6 @@
 Token ) override;
 };
 
-struct PragmaMSOptimizeHandler : public PragmaHandler {
-  PragmaMSOptimizeHandler() : PragmaHandler("optimize") {}
-  void HandlePragma(Preprocessor , PragmaIntroducer Introducer,
-Token ) override;
-};
-
 // "\#pragma fenv_access (on)".
 struct PragmaMSFenvAccessHandler : public PragmaHandler {
   PragmaMSFenvAccessHandler() : PragmaHandler("fenv_access") {}
@@ -449,12 +443,12 @@
 

[PATCH] D127802: [HLSL] Support HLSL vector initializers

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: aaron.ballman, kuhar, python3kgae, MaskRay.
Herald added subscribers: Anastasia, StephenFan.
Herald added a project: All.
beanz requested review of this revision.
Herald added a project: clang.

In HLSL vectors are ext_vectors in all respects except that they
support a constructor style syntax for initializing vectors. This
change adds a translation of vector constructor arguments into
initializer lists.

This supports two oddities of HLSL syntax:
(1) HLSL vectors support constructor syntax
(2) HLSL vectors are expanded to constituate components in constructors


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127802

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaHLSL/BuiltIns/vector-constructors-erros.hlsl
  clang/test/SemaHLSL/BuiltIns/vector-constructors.hlsl

Index: clang/test/SemaHLSL/BuiltIns/vector-constructors.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/BuiltIns/vector-constructors.hlsl
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s 
+
+typedef float float2 __attribute__((ext_vector_type(2)));
+typedef float float3 __attribute__((ext_vector_type(3)));
+
+[numthreads(1,1,1)]
+void entry() {
+  float2 Vec2 = float2(1.0, 2.0);
+  float3 Vec3 = float3(Vec2, 3.0);
+  float3 Vec3b = float3(1.0, 2.0, 3.0);
+}
+
+// For the float2 vector, we just expect a conversion from constructor
+// parameters to an initialization list
+// CHECK: VarDecl 0x{{[0-9a-fA-F]+}}  col:10 used Vec2 'float2':'float __attribute__((ext_vector_type(2)))' cinit
+// CHECK-NEXT: CXXFunctionalCastExpr 0x{{[0-9a-fA-F]+}}  'float2':'float __attribute__((ext_vector_type(2)))' functional cast to float2 
+// CHECK-NEXT: InitListExpr 0x{{[0-9a-fA-F]+}}  'float2':'float __attribute__((ext_vector_type(2)))'
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-fA-F]+}}  'float' 
+// CHECK-NEXT: FloatingLiteral 0x{{[0-9a-fA-F]+}}  'double' 1.00e+00
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-fA-F]+}}  'float' 
+// CHECK-NEXT: FloatingLiteral 0x{{[0-9a-fA-F]+}}  'double' 2.00e+00
+
+
+// For the float 3 things get fun...
+// Here we expect accesses to the vec2 to provide the first and second
+// components using ArraySubscriptExpr
+// CHECK: VarDecl 0x{{[0-9a-fA-F]+}}  col:10 Vec3 'float3':'float __attribute__((ext_vector_type(3)))' cinit
+// CHECK-NEXT: CXXFunctionalCastExpr 0x{{[0-9a-fA-F]+}}  'float3':'float __attribute__((ext_vector_type(3)))' functional cast to float3 
+// CHECK-NEXT: InitListExpr 0x{{[0-9a-fA-F]+}}  'float3':'float __attribute__((ext_vector_type(3)))'
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-fA-F]+}} > 'float' 
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9a-fA-F]+}} > 'float' lvalue
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-fA-F]+}}  'float2':'float __attribute__((ext_vector_type(2)))' lvalue Var 0x{{[0-9a-fA-F]+}} 'Vec2' 'float2':'float __attribute__((ext_vector_type(2)))'
+// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-fA-F]+}} <> 'int' 0
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-fA-F]+}} > 'float' 
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9a-fA-F]+}} > 'float' lvalue
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-fA-F]+}}  'float2':'float __attribute__((ext_vector_type(2)))' lvalue Var 0x{{[0-9a-fA-F]+}} 'Vec2' 'float2':'float __attribute__((ext_vector_type(2)))'
+// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-fA-F]+}} <> 'int' 1
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-fA-F]+}}  'float' 
+// CHECK-NEXT: FloatingLiteral 0x{{[0-9a-fA-F]+}}  'double' 3.00e+00
+
+// CHECK: VarDecl 0x{{[0-9a-fA-F]+}}  col:10 Vec3b 'float3':'float __attribute__((ext_vector_type(3)))' cinit
+// CHECK-NEXT: CXXFunctionalCastExpr 0x{{[0-9a-fA-F]+}}  'float3':'float __attribute__((ext_vector_type(3)))' functional cast to float3 
+// CHECK-NEXT: InitListExpr 0x{{[0-9a-fA-F]+}}  'float3':'float __attribute__((ext_vector_type(3)))'
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-fA-F]+}}  'float' 
+// CHECK-NEXT: FloatingLiteral 0x{{[0-9a-fA-F]+}}  'double' 1.00e+00
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-fA-F]+}}  'float' 
+// CHECK-NEXT: FloatingLiteral 0x{{[0-9a-fA-F]+}}  'double' 2.00e+00
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-fA-F]+}}  'float' 
+// CHECK-NEXT: FloatingLiteral 0x{{[0-9a-fA-F]+}}  'double' 3.00e+00
Index: clang/test/SemaHLSL/BuiltIns/vector-constructors-erros.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/BuiltIns/vector-constructors-erros.hlsl
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -fsyntax-only -verify %s
+
+typedef float float2 __attribute__((ext_vector_type(2)));
+typedef float float3 __attribute__((ext_vector_type(3)));
+
+[numthreads(1,1,1)]
+void entry() {
+  float2 LilVec = float2(1.0, 2.0);
+  float2 BrokenVec = float2(1.0, 2.0, 3.0); // expected-error{{excess elements in vector initializer}}
+  float3 

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Maybe we should document that zero-size arrays aren't treated as flexible 
arrays, since that's what the zero-size array extension was originally designed 
for.

Looks fine otherwise.


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

https://reviews.llvm.org/D126864

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

[PATCH] D127799: [StaticAnalyzer] Remove dead code (DO NOT COMMIT)

[Kazu Hirata via Phabricator via cfe-commits]

kazu created this revision.
Herald added subscribers: abrachet, manas, ASDenysPetrov, martong, dkrupp, 
donat.nagy, Szelethus, a.sidorin, zzheng, szepet, baloghadamsoftware.
Herald added a project: All.
kazu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

DO NOT COMMIT!  This untested patch deletes dead code under
clang/lib/StaticAnalyzer, including debug functions.  Please
intelligently choose what to actually remove.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127799

Files:
  clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
  clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/Iterator.h
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -352,7 +352,6 @@
 
 private:
   void storeTopLevelDecls(DeclGroupRef DG);
-  std::string getFunctionName(const Decl *D);
 
   /// Check if we should skip (not analyze) the given function.
   AnalysisMode getModeForDecl(Decl *D, AnalysisMode Mode);
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -1352,10 +1352,6 @@
   S, SVB.evalUnaryOp(State, S->getOpcode(), Op, S->getType()));
 }
 
-SVal VisitSymExpr(SymbolRef S) { return nonloc::SymbolVal(S); }
-
-SVal VisitMemRegion(const MemRegion *R) { return loc::MemRegionVal(R); }
-
 SVal VisitNonLocSymbolVal(nonloc::SymbolVal V) {
   // Simplification is much more costly than computing complexity.
   // For high complexity, it may be not worth it.
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -107,8 +107,6 @@
 return P.getOpaqueValue() == X.P.getOpaqueValue() &&
Data == X.Data;
   }
-
-  LLVM_DUMP_METHOD void dump() const;
 };
 } // end anonymous namespace
 
@@ -136,7 +134,6 @@
 } // namespace llvm
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
-void BindingKey::dump() const { llvm::errs() << *this; }
 #endif
 
 //===--===//
@@ -255,8 +252,6 @@
   Out << NL;
 }
   }
-
-  LLVM_DUMP_METHOD void dump() const { printJson(llvm::errs()); }
 };
 } // end anonymous namespace
 
@@ -732,10 +727,6 @@
 return true;
   }
 
-  bool AddToWorkList(const MemRegion *R) {
-return static_cast(this)->AddToWorkList(R);
-  }
-
   void RunWorkList() {
 while (!WL.empty()) {
   WorkListElement E = WL.pop_back_val();
@@ -746,7 +737,6 @@
   }
 
   void VisitAddedToCluster(const MemRegion *baseR, const ClusterBindings ) {}
-  void VisitCluster(const MemRegion *baseR, const ClusterBindings *C) {}
 
   void VisitCluster(const MemRegion *BaseR, const ClusterBindings *C,
 bool Flag) {
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -954,8 +954,6 @@
   LLVM_NODISCARD inline ProgramStateRef
   markDisequal(RangeSet::Factory , ProgramStateRef State,
EquivalenceClass Other) const;
-  LLVM_NODISCARD static inline ClassSet
-  getDisequalClasses(ProgramStateRef State, SymbolRef Sym);
   LLVM_NODISCARD inline ClassSet
   

[clang] 657e954 - [clang] Add tests for statement expression in initializers

[Anders Waldenborg via cfe-commits]

Author: Anders Waldenborg
Date: 2022-06-14T23:16:41+02:00
New Revision: 657e954939c8a7d33c40291ef3214333a1e2ca5b

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

LOG: [clang] Add tests for statement expression in initializers

The commit 683e83c5
  [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr
fixed a code generation bug when using (C-extension) statement
expressions inside initializer expressions.

Before that commit a nested static initializer inside the statement
expression would not be emitted, causing it to be zero initialized.

It is a bit surprising (at least to me) that a commit implementing a new
C++ feature would fix this code generation bug. Zooming in it is the
change done in ExprConstant.cpp that helps. That changes so that
"ESR_Failed" is returned in more cases, causing the expression to not be
deemed constant. This fixes the code generation as instead the compiler
has to resort to generating a dynamic initializer.

That commit also meant that some statement expressions (in particular
the ones using static variables) that previously were accepted now are
errors due to not being constant (matching GCC behavior).

Given how a seemingly unrelated change caused this behavior to change,
it is probably a good thing to add at least some rudimentary tests for
these kind expressions.

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

Added: 
clang/test/CodeGen/stmtexpr-init.c
clang/test/Sema/stmtexpr-init.c

Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8766adb27a09a..7d84933b43bca 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -170,6 +170,11 @@ Bug Fixes
   fixes `Issue 48230 `_.
 - Fixed memory leak due to ``VarTemplateSpecializationDecl`` using
   ``TemplateArgumentListInfo`` instead of ``ASTTemplateArgumentListInfo``.
+- An initializer for a static variable declaration, which is nested
+  inside a statement expression in an aggregate initializer, is now
+  emitted as a dynamic initializer. Previously the variable would
+  incorrectly be zero-initialized. In contexts where a dynamic
+  initializer is not allowed this is now diagnosed as an error.
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/test/CodeGen/stmtexpr-init.c 
b/clang/test/CodeGen/stmtexpr-init.c
new file mode 100644
index 0..e2db75518e9a4
--- /dev/null
+++ b/clang/test/CodeGen/stmtexpr-init.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+
+void escape(const void *);
+
+// CHECK-DAG: internal global i8 99
+
+void T1(void) {
+  const char *x[1] = {({static char _x = 99; &_x; })};
+  escape(x);
+}
+
+struct sized_array {
+  int count;
+  int entries[];
+};
+
+#define N_ARGS(...) (sizeof((int[]){__VA_ARGS__}) / sizeof(int))
+
+#define ARRAY_PTR(...) ({\
+  static const struct sized_array _a = {N_ARGS(__VA_ARGS__), {__VA_ARGS__}}; \
+  &_a;   \
+})
+
+struct outer {
+  const struct sized_array *a;
+};
+
+void T2(void) {
+  // CHECK-DAG: internal constant { i32, [2 x i32] } { i32 2, [2 x i32] [i32 
50, i32 60] }
+  const struct sized_array *A = ARRAY_PTR(50, 60);
+
+  // CHECK-DAG: internal constant { i32, [3 x i32] } { i32 3, [3 x i32] [i32 
10, i32 20, i32 30] }
+  struct outer X = {ARRAY_PTR(10, 20, 30)};
+
+  escape(A);
+  escape();
+}

diff  --git a/clang/test/Sema/stmtexpr-init.c b/clang/test/Sema/stmtexpr-init.c
new file mode 100644
index 0..73c87e0d4b726
--- /dev/null
+++ b/clang/test/Sema/stmtexpr-init.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+static int *z[1] = {({ static int _x = 70; &_x; })}; // expected-error 
{{statement expression not allowed at file scope}}
+
+void T1(void) {
+  int *x[1] = {({ static int _x = 10; &_x; })}; // expected-no-error
+
+  /* Before commit
+ 683e83c5 [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr
+ (i.e in clang-14 and earlier)
+ this was silently accepted, but generated incorrect code.
+  */
+  static int *y[1] = {({ static int _x = 20; &_x; })}; // expected-error 
{{initializer element is not a compile-time constant}}
+}



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

[PATCH] D127201: [clang] Add tests for statement expression in initializers

[Anders Waldenborg 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 rG657e954939c8: [clang] Add tests for statement expression in 
initializers (authored by wanders).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127201

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/CodeGen/stmtexpr-init.c
  clang/test/Sema/stmtexpr-init.c


Index: clang/test/Sema/stmtexpr-init.c
===
--- /dev/null
+++ clang/test/Sema/stmtexpr-init.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+static int *z[1] = {({ static int _x = 70; &_x; })}; // expected-error 
{{statement expression not allowed at file scope}}
+
+void T1(void) {
+  int *x[1] = {({ static int _x = 10; &_x; })}; // expected-no-error
+
+  /* Before commit
+ 683e83c5 [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr
+ (i.e in clang-14 and earlier)
+ this was silently accepted, but generated incorrect code.
+  */
+  static int *y[1] = {({ static int _x = 20; &_x; })}; // expected-error 
{{initializer element is not a compile-time constant}}
+}
Index: clang/test/CodeGen/stmtexpr-init.c
===
--- /dev/null
+++ clang/test/CodeGen/stmtexpr-init.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+
+void escape(const void *);
+
+// CHECK-DAG: internal global i8 99
+
+void T1(void) {
+  const char *x[1] = {({static char _x = 99; &_x; })};
+  escape(x);
+}
+
+struct sized_array {
+  int count;
+  int entries[];
+};
+
+#define N_ARGS(...) (sizeof((int[]){__VA_ARGS__}) / sizeof(int))
+
+#define ARRAY_PTR(...) ({\
+  static const struct sized_array _a = {N_ARGS(__VA_ARGS__), {__VA_ARGS__}}; \
+  &_a;   \
+})
+
+struct outer {
+  const struct sized_array *a;
+};
+
+void T2(void) {
+  // CHECK-DAG: internal constant { i32, [2 x i32] } { i32 2, [2 x i32] [i32 
50, i32 60] }
+  const struct sized_array *A = ARRAY_PTR(50, 60);
+
+  // CHECK-DAG: internal constant { i32, [3 x i32] } { i32 3, [3 x i32] [i32 
10, i32 20, i32 30] }
+  struct outer X = {ARRAY_PTR(10, 20, 30)};
+
+  escape(A);
+  escape();
+}
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -170,6 +170,11 @@
   fixes `Issue 48230 `_.
 - Fixed memory leak due to ``VarTemplateSpecializationDecl`` using
   ``TemplateArgumentListInfo`` instead of ``ASTTemplateArgumentListInfo``.
+- An initializer for a static variable declaration, which is nested
+  inside a statement expression in an aggregate initializer, is now
+  emitted as a dynamic initializer. Previously the variable would
+  incorrectly be zero-initialized. In contexts where a dynamic
+  initializer is not allowed this is now diagnosed as an error.
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/Sema/stmtexpr-init.c
===
--- /dev/null
+++ clang/test/Sema/stmtexpr-init.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+static int *z[1] = {({ static int _x = 70; &_x; })}; // expected-error {{statement expression not allowed at file scope}}
+
+void T1(void) {
+  int *x[1] = {({ static int _x = 10; &_x; })}; // expected-no-error
+
+  /* Before commit
+ 683e83c5 [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr
+ (i.e in clang-14 and earlier)
+ this was silently accepted, but generated incorrect code.
+  */
+  static int *y[1] = {({ static int _x = 20; &_x; })}; // expected-error {{initializer element is not a compile-time constant}}
+}
Index: clang/test/CodeGen/stmtexpr-init.c
===
--- /dev/null
+++ clang/test/CodeGen/stmtexpr-init.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+
+void escape(const void *);
+
+// CHECK-DAG: internal global i8 99
+
+void T1(void) {
+  const char *x[1] = {({static char _x = 99; &_x; })};
+  escape(x);
+}
+
+struct sized_array {
+  int count;
+  int entries[];
+};
+
+#define N_ARGS(...) (sizeof((int[]){__VA_ARGS__}) / sizeof(int))
+
+#define ARRAY_PTR(...) ({\
+  static const struct sized_array _a = {N_ARGS(__VA_ARGS__), {__VA_ARGS__}}; \
+  &_a;   \
+})
+
+struct outer {
+  const struct sized_array *a;
+};
+
+void T2(void) {
+  // CHECK-DAG: internal constant { i32, [2 x i32] } { i32 2, [2 x i32] [i32 50, i32 60] }
+  const struct sized_array *A 

[PATCH] D127798: [AArch64] Define __ARM_FEATURE_RCPC

[Mingming Liu via Phabricator via cfe-commits]

mingmingl updated this revision to Diff 436940.
mingmingl added a comment.

update commit message with differential revision link.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127798

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/test/Preprocessor/aarch64-target-features.c


Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -19,6 +19,7 @@
 // CHECK-NOT: __ARM_FEATURE_BIG_ENDIAN
 // CHECK: __ARM_FEATURE_CLZ 1
 // CHECK-NOT: __ARM_FEATURE_CRC32 1
+// CHECK-NOT: __ARM_FEATURE_RCPC 1
 // CHECK-NOT: __ARM_FEATURE_CRYPTO 1
 // CHECK: __ARM_FEATURE_DIRECTED_ROUNDING 1
 // CHECK: __ARM_FEATURE_DIV 1
@@ -561,3 +562,6 @@
 // CHECK-BEFORE-V83-NOT: __ARM_FEATURE_COMPLEX 1
 // CHECK-BEFORE-V83-NOT: __ARM_FEATURE_JCVT 1
 // CHECK-BEFORE-V85-NOT: __ARM_FEATURE_FRINT 1
+
+// RUN: %clang -target aarch64-none-linux-gnu -march=armv8.2-a+rcpc -x c -E 
-dM %s -o - | FileCheck --check-prefix=CHECK-RCPC %s
+// CHECK-RCPC: __ARM_FEATURE_RCPC 1
Index: clang/lib/Basic/Targets/AArch64.h
===
--- clang/lib/Basic/Targets/AArch64.h
+++ clang/lib/Basic/Targets/AArch64.h
@@ -54,6 +54,7 @@
   bool HasLSE;
   bool HasFlagM;
   bool HasMOPS;
+  bool HasRCPC;
 
   llvm::AArch64::ArchKind ArchKind;
 
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -341,6 +341,9 @@
   if (HasCRC)
 Builder.defineMacro("__ARM_FEATURE_CRC32", "1");
 
+  if (HasRCPC)
+Builder.defineMacro("__ARM_FEATURE_RCPC", "1");
+
   // The __ARM_FEATURE_CRYPTO is deprecated in favor of finer grained feature
   // macros for AES, SHA2, SHA3 and SM4
   if (HasAES && HasSHA2)
@@ -544,6 +547,7 @@
   HasMatmulFP32 = false;
   HasLSE = false;
   HasMOPS = false;
+  HasRCPC = false;
 
   ArchKind = llvm::AArch64::ArchKind::INVALID;
 
@@ -593,6 +597,8 @@
 }
 if (Feature == "+crc")
   HasCRC = true;
+if (Feature == "+rcpc")
+  HasRCPC = true;
 if (Feature == "+aes")
   HasAES = true;
 if (Feature == "+sha2")


Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -19,6 +19,7 @@
 // CHECK-NOT: __ARM_FEATURE_BIG_ENDIAN
 // CHECK: __ARM_FEATURE_CLZ 1
 // CHECK-NOT: __ARM_FEATURE_CRC32 1
+// CHECK-NOT: __ARM_FEATURE_RCPC 1
 // CHECK-NOT: __ARM_FEATURE_CRYPTO 1
 // CHECK: __ARM_FEATURE_DIRECTED_ROUNDING 1
 // CHECK: __ARM_FEATURE_DIV 1
@@ -561,3 +562,6 @@
 // CHECK-BEFORE-V83-NOT: __ARM_FEATURE_COMPLEX 1
 // CHECK-BEFORE-V83-NOT: __ARM_FEATURE_JCVT 1
 // CHECK-BEFORE-V85-NOT: __ARM_FEATURE_FRINT 1
+
+// RUN: %clang -target aarch64-none-linux-gnu -march=armv8.2-a+rcpc -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-RCPC %s
+// CHECK-RCPC: __ARM_FEATURE_RCPC 1
Index: clang/lib/Basic/Targets/AArch64.h
===
--- clang/lib/Basic/Targets/AArch64.h
+++ clang/lib/Basic/Targets/AArch64.h
@@ -54,6 +54,7 @@
   bool HasLSE;
   bool HasFlagM;
   bool HasMOPS;
+  bool HasRCPC;
 
   llvm::AArch64::ArchKind ArchKind;
 
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -341,6 +341,9 @@
   if (HasCRC)
 Builder.defineMacro("__ARM_FEATURE_CRC32", "1");
 
+  if (HasRCPC)
+Builder.defineMacro("__ARM_FEATURE_RCPC", "1");
+
   // The __ARM_FEATURE_CRYPTO is deprecated in favor of finer grained feature
   // macros for AES, SHA2, SHA3 and SM4
   if (HasAES && HasSHA2)
@@ -544,6 +547,7 @@
   HasMatmulFP32 = false;
   HasLSE = false;
   HasMOPS = false;
+  HasRCPC = false;
 
   ArchKind = llvm::AArch64::ArchKind::INVALID;
 
@@ -593,6 +597,8 @@
 }
 if (Feature == "+crc")
   HasCRC = true;
+if (Feature == "+rcpc")
+  HasRCPC = true;
 if (Feature == "+aes")
   HasAES = true;
 if (Feature == "+sha2")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127798: [AArch64] Define __ARM_FEATURE_RCPC

[Mingming Liu via Phabricator via cfe-commits]

mingmingl created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
mingmingl requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch implements the definition of __ARM_FEATURE_RCPC when clang commands 
specifies +rcpc.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127798

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/test/Preprocessor/aarch64-target-features.c


Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -19,6 +19,7 @@
 // CHECK-NOT: __ARM_FEATURE_BIG_ENDIAN
 // CHECK: __ARM_FEATURE_CLZ 1
 // CHECK-NOT: __ARM_FEATURE_CRC32 1
+// CHECK-NOT: __ARM_FEATURE_RCPC 1
 // CHECK-NOT: __ARM_FEATURE_CRYPTO 1
 // CHECK: __ARM_FEATURE_DIRECTED_ROUNDING 1
 // CHECK: __ARM_FEATURE_DIV 1
@@ -561,3 +562,6 @@
 // CHECK-BEFORE-V83-NOT: __ARM_FEATURE_COMPLEX 1
 // CHECK-BEFORE-V83-NOT: __ARM_FEATURE_JCVT 1
 // CHECK-BEFORE-V85-NOT: __ARM_FEATURE_FRINT 1
+
+// RUN: %clang -target aarch64-none-linux-gnu -march=armv8.2-a+rcpc -x c -E 
-dM %s -o - | FileCheck --check-prefix=CHECK-RCPC %s
+// CHECK-RCPC: __ARM_FEATURE_RCPC 1
Index: clang/lib/Basic/Targets/AArch64.h
===
--- clang/lib/Basic/Targets/AArch64.h
+++ clang/lib/Basic/Targets/AArch64.h
@@ -54,6 +54,7 @@
   bool HasLSE;
   bool HasFlagM;
   bool HasMOPS;
+  bool HasRCPC;
 
   llvm::AArch64::ArchKind ArchKind;
 
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -341,6 +341,9 @@
   if (HasCRC)
 Builder.defineMacro("__ARM_FEATURE_CRC32", "1");
 
+  if (HasRCPC)
+Builder.defineMacro("__ARM_FEATURE_RCPC", "1");
+
   // The __ARM_FEATURE_CRYPTO is deprecated in favor of finer grained feature
   // macros for AES, SHA2, SHA3 and SM4
   if (HasAES && HasSHA2)
@@ -544,6 +547,7 @@
   HasMatmulFP32 = false;
   HasLSE = false;
   HasMOPS = false;
+  HasRCPC = false;
 
   ArchKind = llvm::AArch64::ArchKind::INVALID;
 
@@ -593,6 +597,8 @@
 }
 if (Feature == "+crc")
   HasCRC = true;
+if (Feature == "+rcpc")
+  HasRCPC = true;
 if (Feature == "+aes")
   HasAES = true;
 if (Feature == "+sha2")


Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -19,6 +19,7 @@
 // CHECK-NOT: __ARM_FEATURE_BIG_ENDIAN
 // CHECK: __ARM_FEATURE_CLZ 1
 // CHECK-NOT: __ARM_FEATURE_CRC32 1
+// CHECK-NOT: __ARM_FEATURE_RCPC 1
 // CHECK-NOT: __ARM_FEATURE_CRYPTO 1
 // CHECK: __ARM_FEATURE_DIRECTED_ROUNDING 1
 // CHECK: __ARM_FEATURE_DIV 1
@@ -561,3 +562,6 @@
 // CHECK-BEFORE-V83-NOT: __ARM_FEATURE_COMPLEX 1
 // CHECK-BEFORE-V83-NOT: __ARM_FEATURE_JCVT 1
 // CHECK-BEFORE-V85-NOT: __ARM_FEATURE_FRINT 1
+
+// RUN: %clang -target aarch64-none-linux-gnu -march=armv8.2-a+rcpc -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-RCPC %s
+// CHECK-RCPC: __ARM_FEATURE_RCPC 1
Index: clang/lib/Basic/Targets/AArch64.h
===
--- clang/lib/Basic/Targets/AArch64.h
+++ clang/lib/Basic/Targets/AArch64.h
@@ -54,6 +54,7 @@
   bool HasLSE;
   bool HasFlagM;
   bool HasMOPS;
+  bool HasRCPC;
 
   llvm::AArch64::ArchKind ArchKind;
 
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -341,6 +341,9 @@
   if (HasCRC)
 Builder.defineMacro("__ARM_FEATURE_CRC32", "1");
 
+  if (HasRCPC)
+Builder.defineMacro("__ARM_FEATURE_RCPC", "1");
+
   // The __ARM_FEATURE_CRYPTO is deprecated in favor of finer grained feature
   // macros for AES, SHA2, SHA3 and SM4
   if (HasAES && HasSHA2)
@@ -544,6 +547,7 @@
   HasMatmulFP32 = false;
   HasLSE = false;
   HasMOPS = false;
+  HasRCPC = false;
 
   ArchKind = llvm::AArch64::ArchKind::INVALID;
 
@@ -593,6 +597,8 @@
 }
 if (Feature == "+crc")
   HasCRC = true;
+if (Feature == "+rcpc")
+  HasRCPC = true;
 if (Feature == "+aes")
   HasAES = true;
 if (Feature == "+sha2")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

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

rsmith added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:9265-9266
+  ? Context.getBitIntType(!IsMakeSigned, Context.getIntWidth(BaseType))
+  : Context.getIntTypeForBitwidth(Context.getIntWidth(BaseType),
+  IsMakeSigned);
+  Qualifiers Quals = Underlying.getQualifiers();

rsmith wrote:
> cjdb wrote:
> > rsmith wrote:
> > > This is wrong: if, say, `int` and `long` are the same bit-width, this 
> > > will give the same type for `__make_unsigned(int)` and 
> > > `__make_unsigned(long)`, where they are required to produce `unsigned 
> > > int` and `unsigned long` respectively.
> > > 
> > > Please look at `Context.getCorrespondingSignedType` and 
> > > `Context.getCorrespondingUnsignedType` to see how to do this properly; 
> > > you may be able to call those functions directly in some cases, but be 
> > > careful about the cases where we're required to return the lowest-rank 
> > > int type of the given size. Note that `getIntTypeForBitwidth` does *not* 
> > > do that; rather, it produces the *preferred* type of the given width, and 
> > > for WebAssembly and AVR it produces something other than the lowest-rank 
> > > type in practice in some cases.
> > This makes me very happy.
> The comment on this producing the wrong underlying type for enums in some 
> cases doesn't seem to be addressed. You need to produce the lowest-rank type 
> of the given size, which is not what `getIntTypeForBitwidth` does.
Specifically, the rule you need to implement for enums is that the resulting 
type "names the [un]signed integer type with smallest rank (6.8.5) for which 
sizeof(T) == sizeof(type)". So the first of `[un]signed char`, `[un]signed 
short`, `[un]signed int`, `[un]signed long`, `[un]signed long long` that is the 
same size as the enum, and if none of those work then an extended integer type.

For example, if `int` and `long` are the same size, then given `enum E : long 
{};`, `__make_unsigned(E)` is required to produce `unsigned int`, not `unsigned 
long`. This is not what `getIntTypeForBitwidth` does in general; rather, it 
answers the target-specific question of "which integer type would you like to 
be used for an integer that is N bits wide?".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

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

rsmith added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:5863
   // FIXME: derive from "Target" ?
-  return WCharTy;
+  return IntTy;
 }

This change seems surprising. Can you explain what's going on here?



Comment at: clang/lib/AST/ASTContext.cpp:10712-10713
 QualType ASTContext::getCorrespondingUnsignedType(QualType T) const {
-  assert((T->hasSignedIntegerRepresentation() || T->isSignedFixedPointType()) 
&&
+  assert((T->hasSignedIntegerRepresentation() || T->isIntegerType() ||
+  T->isEnumeralType() || T->isSignedFixedPointType()) &&
  "Unexpected type");

If we now allow unsigned types to be passed in here, can we do so consistently?

This seems to do the wrong thing for a vector of scoped enumeration type. Is 
that a problem for the builtins?



Comment at: clang/lib/AST/ASTContext.cpp:10731-10759
+  case BuiltinType::Char_U:
   case BuiltinType::Char_S:
   case BuiltinType::SChar:
+  case BuiltinType::UChar:
+  case BuiltinType::Char8:
 return UnsignedCharTy;
   case BuiltinType::Short:

(Continuation of suggested edits in comment a few lines below.)



Comment at: clang/lib/AST/ASTContext.cpp:10785-10786
 return SatUnsignedLongFractTy;
   default:
-llvm_unreachable("Unexpected signed integer or fixed point type");
+llvm_unreachable("Unexpected integer or signed fixed point type");
   }

Perhaps we can handle all the plain unsigned types (other than `wchar_t` and 
`char`, which are special) here? This would also cover the fixed-point types.



Comment at: clang/lib/AST/ASTContext.cpp:10791
 QualType ASTContext::getCorrespondingSignedType(QualType T) const {
-  assert((T->hasUnsignedIntegerRepresentation() ||
-  T->isUnsignedFixedPointType()) &&
+  assert((T->hasUnsignedIntegerRepresentation() || T->isIntegerType() ||
+  T->isEnumeralType() || T->isUnsignedFixedPointType()) &&

Similar comments for this function as previous one.



Comment at: clang/lib/AST/TypePrinter.cpp:1158
raw_ostream ) {
   IncludeStrongLifetimeRAII Strong(Policy);
 }

Remove this line too. No point building an RAII scope with nothing in it.



Comment at: clang/lib/Parse/ParseExpr.cpp:1750
+#undef TRANSFORM_TYPE_TRAIT_DEF
+if (NextToken().is(tok::less)) {
+  Tok.setKind(tok::identifier);

cjdb wrote:
> rsmith wrote:
> > Here you're checking for `trait<` and treating it as an identifier; 
> > elsewhere you're checking for `trait(` and treating it as a builtin. Is 
> > there a reason to treat `trait` followed by a token other than `(` or `<` 
> > inconsistently?
> The parser needs to treat `trait<` as an identifier to accommodate 
> libstdc++'s usage of a few of these as alias templates. I've added a comment 
> to explain why in the code.
I agree we need to treat `trait<` as an identifier and `trait(` as the builtin. 
My question is, why do we have inconsistent treatment of the remaining cases 
(`trait` followed by any other token)? For example, 
`MaybeParseTypeTransformTypeSpecifier` treats `trait + 1` as an identifier. But 
this code treats it as the builtin name.

I think there are two choices that make the most sense, if they work:

1) `trait(` is the builtin and any other utterance is an identifier, or
2) `trait<` is an identifier, `using trait =` is an identifier, and any other 
utterance is the builtin.

I think I prefer option (2), given that it's defining the narrower special 
case. But all I'm really looking for here is a consistent story one way or the 
other, if it's feasible to have one.



Comment at: clang/lib/Sema/SemaType.cpp:9160-9164
+  Qualifiers Quals = Pointee.getQualifiers();
+  Quals.removeAddressSpace();
+  return Context.getUnaryTransformType(
+  BaseType,
+  QualType(Pointee.getSplitUnqualifiedType().Ty, Quals.getAsOpaqueValue()),

You're splitting the type into unqualified type and quailfiers twice here, once 
in the call to `getQualifiers` and again in `getSplitUnqualifiedType`. It'd be 
better to only split it once.



Comment at: clang/lib/Sema/SemaType.cpp:9161
+  Qualifiers Quals = Pointee.getQualifiers();
+  Quals.removeAddressSpace();
+  return Context.getUnaryTransformType(

Do we really want to remove the address space here? The libc++ and libstdc++ 
implementations of the trait don't do that (https://godbolt.org/z/jqafYP6er) 
and it makes the behavior of `__remove_pointer` inconsistent with the behavior 
of `__add_pointer`. Can we just produce the pointee type intact, including all 
of its qualifiers?



Comment at: clang/lib/Sema/SemaType.cpp:9164
+  BaseType,
+  QualType(Pointee.getSplitUnqualifiedType().Ty, Quals.getAsOpaqueValue()),
+  UKind);

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

[Roy Jacobson via Phabricator via cfe-commits]

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

Thank you! LGTM. I see the CI still has those OpenMP failures - could you try 
to rebase on main to get it green?

@erichkeane is this good to go? Should I ping some ABI people about this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

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

[PATCH] D124750: [MLIR] Add a utility to sort the operands of commutative ops

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.



Comment at: mlir/include/mlir/Transforms/CommutativityUtils.h:211
+template 
+class SortCommutativeOperands : public OpRewritePattern {
+  using OpRewritePattern::OpRewritePattern;

Can we make this not a template? This will be a code bloat otherwise.



Comment at: mlir/include/mlir/Transforms/CommutativityUtils.h:332
+
+} // namespace mlir
+

There seems to me to be far too much code in the public head: I can't isolate 
the public API here, if this is just about a pattern then the usual way is to 
have a "populate" method.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124750

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

[PATCH] D127707: [Clang] Simplify unifying target features

[Joseph Huber 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 rGc4a2674e21c4: [Clang] Simplify unifying target features 
(authored by jhuber6).

Changed prior to commit:
  https://reviews.llvm.org/D127707?vs=436621=436900#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127707

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h


Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -173,8 +173,7 @@
llvm::opt::OptSpecifier Group);
 
 /// If there are multiple +xxx or -xxx features, keep the last one.
-std::vector
-unifyTargetFeatures(const std::vector );
+SmallVector unifyTargetFeatures(ArrayRef Features);
 
 /// Handles the -save-stats option and returns the filename to save statistics
 /// to.
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -143,28 +143,16 @@
   }
 }
 
-std::vector
-tools::unifyTargetFeatures(const std::vector ) {
-  std::vector UnifiedFeatures;
-  // Find the last of each feature.
-  llvm::StringMap LastOpt;
-  for (unsigned I = 0, N = Features.size(); I < N; ++I) {
-StringRef Name = Features[I];
-assert(Name[0] == '-' || Name[0] == '+');
-LastOpt[Name.drop_front(1)] = I;
-  }
-
-  for (unsigned I = 0, N = Features.size(); I < N; ++I) {
-// If this feature was overridden, ignore it.
-StringRef Name = Features[I];
-llvm::StringMap::iterator LastI = 
LastOpt.find(Name.drop_front(1));
-assert(LastI != LastOpt.end());
-unsigned Last = LastI->second;
-if (Last != I)
-  continue;
-
-UnifiedFeatures.push_back(Name);
+SmallVector
+tools::unifyTargetFeatures(ArrayRef Features) {
+  // Only add a feature if it hasn't been seen before starting from the end.
+  SmallVector UnifiedFeatures;
+  llvm::DenseSet UsedFeatures;
+  for (StringRef Feature : llvm::reverse(Features)) {
+if (UsedFeatures.insert(Feature.drop_front()).second)
+  UnifiedFeatures.insert(UnifiedFeatures.begin(), Feature);
   }
+
   return UnifiedFeatures;
 }
 


Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -173,8 +173,7 @@
llvm::opt::OptSpecifier Group);
 
 /// If there are multiple +xxx or -xxx features, keep the last one.
-std::vector
-unifyTargetFeatures(const std::vector );
+SmallVector unifyTargetFeatures(ArrayRef Features);
 
 /// Handles the -save-stats option and returns the filename to save statistics
 /// to.
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -143,28 +143,16 @@
   }
 }
 
-std::vector
-tools::unifyTargetFeatures(const std::vector ) {
-  std::vector UnifiedFeatures;
-  // Find the last of each feature.
-  llvm::StringMap LastOpt;
-  for (unsigned I = 0, N = Features.size(); I < N; ++I) {
-StringRef Name = Features[I];
-assert(Name[0] == '-' || Name[0] == '+');
-LastOpt[Name.drop_front(1)] = I;
-  }
-
-  for (unsigned I = 0, N = Features.size(); I < N; ++I) {
-// If this feature was overridden, ignore it.
-StringRef Name = Features[I];
-llvm::StringMap::iterator LastI = LastOpt.find(Name.drop_front(1));
-assert(LastI != LastOpt.end());
-unsigned Last = LastI->second;
-if (Last != I)
-  continue;
-
-UnifiedFeatures.push_back(Name);
+SmallVector
+tools::unifyTargetFeatures(ArrayRef Features) {
+  // Only add a feature if it hasn't been seen before starting from the end.
+  SmallVector UnifiedFeatures;
+  llvm::DenseSet UsedFeatures;
+  for (StringRef Feature : llvm::reverse(Features)) {
+if (UsedFeatures.insert(Feature.drop_front()).second)
+  UnifiedFeatures.insert(UnifiedFeatures.begin(), Feature);
   }
+
   return UnifiedFeatures;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c4a2674 - [Clang] Simplify unifying target features

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2022-06-14T15:58:16-04:00
New Revision: c4a2674e21c4f5ffd46f90982ce3c3d952018e66

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

LOG: [Clang] Simplify unifying target features

This patch simplifies how we unify target features. Now we simply
iterate the input in reverse and only insert the feature if it hasn't
been seen yet. The only reason we need to reverse this at the end is to
keep the features in order for the existing tests.

Reviewed By: tra

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/lib/Driver/ToolChains/CommonArgs.h

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 7be25072c62e6..accf79f767249 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -143,28 +143,16 @@ void tools::handleTargetFeaturesGroup(const ArgList ,
   }
 }
 
-std::vector
-tools::unifyTargetFeatures(const std::vector ) {
-  std::vector UnifiedFeatures;
-  // Find the last of each feature.
-  llvm::StringMap LastOpt;
-  for (unsigned I = 0, N = Features.size(); I < N; ++I) {
-StringRef Name = Features[I];
-assert(Name[0] == '-' || Name[0] == '+');
-LastOpt[Name.drop_front(1)] = I;
-  }
-
-  for (unsigned I = 0, N = Features.size(); I < N; ++I) {
-// If this feature was overridden, ignore it.
-StringRef Name = Features[I];
-llvm::StringMap::iterator LastI = 
LastOpt.find(Name.drop_front(1));
-assert(LastI != LastOpt.end());
-unsigned Last = LastI->second;
-if (Last != I)
-  continue;
-
-UnifiedFeatures.push_back(Name);
+SmallVector
+tools::unifyTargetFeatures(ArrayRef Features) {
+  // Only add a feature if it hasn't been seen before starting from the end.
+  SmallVector UnifiedFeatures;
+  llvm::DenseSet UsedFeatures;
+  for (StringRef Feature : llvm::reverse(Features)) {
+if (UsedFeatures.insert(Feature.drop_front()).second)
+  UnifiedFeatures.insert(UnifiedFeatures.begin(), Feature);
   }
+
   return UnifiedFeatures;
 }
 

diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.h 
b/clang/lib/Driver/ToolChains/CommonArgs.h
index b0238612234c5..3052cbf1a3c38 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.h
+++ b/clang/lib/Driver/ToolChains/CommonArgs.h
@@ -173,8 +173,7 @@ void handleTargetFeaturesGroup(const llvm::opt::ArgList 
,
llvm::opt::OptSpecifier Group);
 
 /// If there are multiple +xxx or -xxx features, keep the last one.
-std::vector
-unifyTargetFeatures(const std::vector );
+SmallVector unifyTargetFeatures(ArrayRef Features);
 
 /// Handles the -save-stats option and returns the filename to save statistics
 /// to.



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

[PATCH] D127759: [Diagnostic] Clarify -Winfinite-recursion message

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:63
 def warn_infinite_recursive_function : Warning<
-  "all paths through this function will call itself">,
+  "In order to understand recursion, you must first understand recursion">,
   InGroup, DefaultIgnore;

erichkeane wrote:
> erichkeane wrote:
> > Codesbyusman wrote:
> > > Codesbyusman wrote:
> > > > aaron.ballman wrote:
> > > > > erichkeane wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > A few things:
> > > > > > > 
> > > > > > > * Diagnostics in Clang are not intended to be grammatically 
> > > > > > > correct, so they don't start with a capital letter or end with a 
> > > > > > > trailing full stop (so if this wording was kept, you should use 
> > > > > > > `in order` instead of `In order`).
> > > > > > > * While this is a cute way to get the point across, we want the 
> > > > > > > diagnostic to help guide the user as to why their code is wrong 
> > > > > > > so they have the best chance to fix the issue.
> > > > > > > 
> > > > > > > I think the original wording isn't too bad here because it's 
> > > > > > > lexically associated with the function definition itself (so the 
> > > > > > > "this function" is clear in situ), but I do think "will call 
> > > > > > > itself" is a bit awkward. Perhaps "this function is called 
> > > > > > > recursively on all paths through the function", but I don't feel 
> > > > > > > strongly either.
> > > > > > I like the suggestion by Aaron here, "function %0 is called 
> > > > > > recursively through all paths through %0" (or the function).  
> > > > > > Perhaps the double %0 is a touch of a recursion joke as well :) 
> > > > > TBH, I'd probably skip the %0 because the warning is issued on the 
> > > > > same line as the function definition itself, so the name should 
> > > > > already be obvious to the user.
> > > > > 
> > > > > This diagnostic doesn't currently handle mutually recursive 
> > > > > functions, and I kind of worry that adding %0 implies we support that 
> > > > > case when we don't.
> > > > **"this function is called recursively on all paths through the 
> > > > function"**  is good. what you say which will be more better as the  
> > > > **"all paths through this function will call itself"**  is also ok. 
> > > if i am not wrong it is indicating the infinite recursion as no base 
> > > case. can we prompt user in some a decent manner that the base case is 
> > > missing. i.e. including some phrase in **"this function is called 
> > > recursively on all paths through the function"** that shows base case was 
> > > missing or this would be a bad idea? any suggestions.
> > I think I'd like the first one, the location data alone is pretty annoying 
> > in many cases.
> >>I think I'd like the first one, the location data alone is pretty annoying 
> >>in many cases.
> 
> Spoke with Aaron offline, our policy is to just use the names when it is 
> ambiguous/helpful in a way that knowing the line is not.  I'm not a huge fan 
> of the policy, but am not going to lobby it here, feel free to use one of hte 
> below suggestions.
> Spoke with Aaron offline, our policy is to just use the names when it is 
> ambiguous/helpful in a way that knowing the line is not. I'm not a huge fan 
> of the policy, but am not going to lobby it here, feel free to use one of hte 
> below suggestions.

FWIW, my feelings are not super strongly held (and it's possible this isn't 
"policy" so much as "the way some folks do things"), so if @Codesbyusman 
decides he wants to use %0 to name the function in the diagnostic or not, 
that's fine by me also. The only bit I definitely feel strongly about is not 
repeating the function name in the diagnostic and avoiding an implication that 
the diagnostic works for detecting mutual recursion. So I'd be fine with 
`function %0 is called recursively on all paths through the function`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127759

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

[PATCH] D124753: [HLSL] Set main as default entry.

[Xiang Li via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGenHLSL/entry_default.hlsl:14
+// CHECK-NOT: "dx.shader"="compute"
+// CHECK-SAM: }
+[numthreads(1, 1, 1)] void main() {

bogner wrote:
> typo, should be `CHECK-SAME`
Good catch.
It is strange that the test will pass even with the typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124753

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

[PATCH] D127641: [clang-cl][MSVC] Enable /Zc:alignedNew for C++17 and /Zc:sizedDealloc by default

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: luken-google.
rnk added a comment.

+@luken-google




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6497
+ options::OPT_fno_aligned_allocation) &&
+LanguageStandard == "-std=c++17")
+  CmdArgs.push_back("-faligned-allocation");

Only for C++17, and not C++17 and above?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6641
   // by default.
   if (Arg *A = Args.getLastArg(options::OPT_faligned_allocation,
options::OPT_fno_aligned_allocation,

steplong wrote:
> There's a check here for faligned-allocation, but I wasn't sure how to get 
> the c++ standard here. Also, I couldn't figure a clean way to compare if the 
> standard was c++17 onwards, so I just enabled -faligned-allocation for C++17 
> above
What I would do is to compute a boolean above and use it as the default value 
here. The -cc1 line should be canonical, it shouldn't have duplicate, 
potentially contradictory flags.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127641

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

[PATCH] D124753: [HLSL] Set main as default entry.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 436898.
python3kgae added a comment.

Fix typo in test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124753

Files:
  clang/include/clang/Driver/Options.td
  clang/test/CodeGenHLSL/entry_default.hlsl


Index: clang/test/CodeGenHLSL/entry_default.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/entry_default.hlsl
@@ -0,0 +1,29 @@
+// RUN: %clang --driver-mode=dxc -Tcs_6_1 -fcgl -Fo - %s | FileCheck %s
+// RUN: %clang --driver-mode=dxc -Efoo -Tcs_6_1 -fcgl -Fo - %s | FileCheck %s 
--check-prefix=NOTMAIN
+
+// Make sure main is default entry.
+// Make sure not mangle entry.
+// CHECK: define void @main() [[MAIN_ATTR:#[0-9]]]
+// CHECK: define void @_Z3foov() [[FOO_ATTR:#[0-9]]]
+// Make sure only main has dx.shader attribute.
+// CHECK: [[MAIN_ATTR]]
+// CHECK-SAME: "dx.shader"="compute"
+// CHECK-SAME: }
+// CHECK: [[FOO_ATTR]]
+// CHECK-NOT: "dx.shader"="compute"
+// CHECK-SAME: }
+[numthreads(1, 1, 1)] void main() {
+
+}
+
+// NOTMAIN: define void @main() [[MAIN_ATTR:#[0-9]]]
+// NOTMAIN: define void @foo() [[FOO_ATTR:#[0-9]]]
+// Make sure only foo has dx.shader attribute.
+// NOTMAIN: [[MAIN_ATTR]]
+// NOTMAIN-NOT:"dx.shader"="compute"
+// NOTMAIN-SAME:}
+// NOTMAIN: [[FOO_ATTR]]
+// NOTMAIN-SAME: "dx.shader"="compute"
+[numthreads(1, 1, 1)] void foo() {
+
+}
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6810,5 +6810,5 @@
 def hlsl_entrypoint : Option<["--", "/", "-"], "E", KIND_JOINED_OR_SEPARATE>,
   Group,
   Flags<[DXCOption, CC1Option, NoXarchOption]>,
-  MarshallingInfoString>,
+  MarshallingInfoString, 
"\"main\"">,
   HelpText<"Entry point name">;


Index: clang/test/CodeGenHLSL/entry_default.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/entry_default.hlsl
@@ -0,0 +1,29 @@
+// RUN: %clang --driver-mode=dxc -Tcs_6_1 -fcgl -Fo - %s | FileCheck %s
+// RUN: %clang --driver-mode=dxc -Efoo -Tcs_6_1 -fcgl -Fo - %s | FileCheck %s --check-prefix=NOTMAIN
+
+// Make sure main is default entry.
+// Make sure not mangle entry.
+// CHECK: define void @main() [[MAIN_ATTR:#[0-9]]]
+// CHECK: define void @_Z3foov() [[FOO_ATTR:#[0-9]]]
+// Make sure only main has dx.shader attribute.
+// CHECK: [[MAIN_ATTR]]
+// CHECK-SAME: "dx.shader"="compute"
+// CHECK-SAME: }
+// CHECK: [[FOO_ATTR]]
+// CHECK-NOT: "dx.shader"="compute"
+// CHECK-SAME: }
+[numthreads(1, 1, 1)] void main() {
+
+}
+
+// NOTMAIN: define void @main() [[MAIN_ATTR:#[0-9]]]
+// NOTMAIN: define void @foo() [[FOO_ATTR:#[0-9]]]
+// Make sure only foo has dx.shader attribute.
+// NOTMAIN: [[MAIN_ATTR]]
+// NOTMAIN-NOT:"dx.shader"="compute"
+// NOTMAIN-SAME:}
+// NOTMAIN: [[FOO_ATTR]]
+// NOTMAIN-SAME: "dx.shader"="compute"
+[numthreads(1, 1, 1)] void foo() {
+
+}
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6810,5 +6810,5 @@
 def hlsl_entrypoint : Option<["--", "/", "-"], "E", KIND_JOINED_OR_SEPARATE>,
   Group,
   Flags<[DXCOption, CC1Option, NoXarchOption]>,
-  MarshallingInfoString>,
+  MarshallingInfoString, "\"main\"">,
   HelpText<"Entry point name">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127759: [Diagnostic] Clarify -Winfinite-recursion message

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:63
 def warn_infinite_recursive_function : Warning<
-  "all paths through this function will call itself">,
+  "In order to understand recursion, you must first understand recursion">,
   InGroup, DefaultIgnore;

erichkeane wrote:
> Codesbyusman wrote:
> > Codesbyusman wrote:
> > > aaron.ballman wrote:
> > > > erichkeane wrote:
> > > > > aaron.ballman wrote:
> > > > > > A few things:
> > > > > > 
> > > > > > * Diagnostics in Clang are not intended to be grammatically 
> > > > > > correct, so they don't start with a capital letter or end with a 
> > > > > > trailing full stop (so if this wording was kept, you should use `in 
> > > > > > order` instead of `In order`).
> > > > > > * While this is a cute way to get the point across, we want the 
> > > > > > diagnostic to help guide the user as to why their code is wrong so 
> > > > > > they have the best chance to fix the issue.
> > > > > > 
> > > > > > I think the original wording isn't too bad here because it's 
> > > > > > lexically associated with the function definition itself (so the 
> > > > > > "this function" is clear in situ), but I do think "will call 
> > > > > > itself" is a bit awkward. Perhaps "this function is called 
> > > > > > recursively on all paths through the function", but I don't feel 
> > > > > > strongly either.
> > > > > I like the suggestion by Aaron here, "function %0 is called 
> > > > > recursively through all paths through %0" (or the function).  Perhaps 
> > > > > the double %0 is a touch of a recursion joke as well :) 
> > > > TBH, I'd probably skip the %0 because the warning is issued on the same 
> > > > line as the function definition itself, so the name should already be 
> > > > obvious to the user.
> > > > 
> > > > This diagnostic doesn't currently handle mutually recursive functions, 
> > > > and I kind of worry that adding %0 implies we support that case when we 
> > > > don't.
> > > **"this function is called recursively on all paths through the 
> > > function"**  is good. what you say which will be more better as the  
> > > **"all paths through this function will call itself"**  is also ok. 
> > if i am not wrong it is indicating the infinite recursion as no base case. 
> > can we prompt user in some a decent manner that the base case is missing. 
> > i.e. including some phrase in **"this function is called recursively on all 
> > paths through the function"** that shows base case was missing or this 
> > would be a bad idea? any suggestions.
> I think I'd like the first one, the location data alone is pretty annoying in 
> many cases.
>>I think I'd like the first one, the location data alone is pretty annoying in 
>>many cases.

Spoke with Aaron offline, our policy is to just use the names when it is 
ambiguous/helpful in a way that knowing the line is not.  I'm not a huge fan of 
the policy, but am not going to lobby it here, feel free to use one of hte 
below suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127759

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

[PATCH] D124753: [HLSL] Set main as default entry.

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: clang/test/CodeGenHLSL/entry_default.hlsl:14
+// CHECK-NOT: "dx.shader"="compute"
+// CHECK-SAM: }
+[numthreads(1, 1, 1)] void main() {

typo, should be `CHECK-SAME`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124753

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

[PATCH] D127759: [Diagnostic] Clarify -Winfinite-recursion message

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:63
 def warn_infinite_recursive_function : Warning<
-  "all paths through this function will call itself">,
+  "In order to understand recursion, you must first understand recursion">,
   InGroup, DefaultIgnore;

Codesbyusman wrote:
> Codesbyusman wrote:
> > aaron.ballman wrote:
> > > erichkeane wrote:
> > > > aaron.ballman wrote:
> > > > > A few things:
> > > > > 
> > > > > * Diagnostics in Clang are not intended to be grammatically correct, 
> > > > > so they don't start with a capital letter or end with a trailing full 
> > > > > stop (so if this wording was kept, you should use `in order` instead 
> > > > > of `In order`).
> > > > > * While this is a cute way to get the point across, we want the 
> > > > > diagnostic to help guide the user as to why their code is wrong so 
> > > > > they have the best chance to fix the issue.
> > > > > 
> > > > > I think the original wording isn't too bad here because it's 
> > > > > lexically associated with the function definition itself (so the 
> > > > > "this function" is clear in situ), but I do think "will call itself" 
> > > > > is a bit awkward. Perhaps "this function is called recursively on all 
> > > > > paths through the function", but I don't feel strongly either.
> > > > I like the suggestion by Aaron here, "function %0 is called recursively 
> > > > through all paths through %0" (or the function).  Perhaps the double %0 
> > > > is a touch of a recursion joke as well :) 
> > > TBH, I'd probably skip the %0 because the warning is issued on the same 
> > > line as the function definition itself, so the name should already be 
> > > obvious to the user.
> > > 
> > > This diagnostic doesn't currently handle mutually recursive functions, 
> > > and I kind of worry that adding %0 implies we support that case when we 
> > > don't.
> > **"this function is called recursively on all paths through the function"** 
> >  is good. what you say which will be more better as the  **"all paths 
> > through this function will call itself"**  is also ok. 
> if i am not wrong it is indicating the infinite recursion as no base case. 
> can we prompt user in some a decent manner that the base case is missing. 
> i.e. including some phrase in **"this function is called recursively on all 
> paths through the function"** that shows base case was missing or this would 
> be a bad idea? any suggestions.
I think I'd like the first one, the location data alone is pretty annoying in 
many cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127759

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

[PATCH] D127759: [Diagnostic] Clarify -Winfinite-recursion message

[Muhammad Usman Shahid via Phabricator via cfe-commits]

Codesbyusman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:63
 def warn_infinite_recursive_function : Warning<
-  "all paths through this function will call itself">,
+  "In order to understand recursion, you must first understand recursion">,
   InGroup, DefaultIgnore;

aaron.ballman wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > A few things:
> > > 
> > > * Diagnostics in Clang are not intended to be grammatically correct, so 
> > > they don't start with a capital letter or end with a trailing full stop 
> > > (so if this wording was kept, you should use `in order` instead of `In 
> > > order`).
> > > * While this is a cute way to get the point across, we want the 
> > > diagnostic to help guide the user as to why their code is wrong so they 
> > > have the best chance to fix the issue.
> > > 
> > > I think the original wording isn't too bad here because it's lexically 
> > > associated with the function definition itself (so the "this function" is 
> > > clear in situ), but I do think "will call itself" is a bit awkward. 
> > > Perhaps "this function is called recursively on all paths through the 
> > > function", but I don't feel strongly either.
> > I like the suggestion by Aaron here, "function %0 is called recursively 
> > through all paths through %0" (or the function).  Perhaps the double %0 is 
> > a touch of a recursion joke as well :) 
> TBH, I'd probably skip the %0 because the warning is issued on the same line 
> as the function definition itself, so the name should already be obvious to 
> the user.
> 
> This diagnostic doesn't currently handle mutually recursive functions, and I 
> kind of worry that adding %0 implies we support that case when we don't.
**"this function is called recursively on all paths through the function"**  is 
good. what you say which will be more better as the  **"all paths through this 
function will call itself"**  is also ok. 



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:63
 def warn_infinite_recursive_function : Warning<
-  "all paths through this function will call itself">,
+  "In order to understand recursion, you must first understand recursion">,
   InGroup, DefaultIgnore;

Codesbyusman wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > A few things:
> > > > 
> > > > * Diagnostics in Clang are not intended to be grammatically correct, so 
> > > > they don't start with a capital letter or end with a trailing full stop 
> > > > (so if this wording was kept, you should use `in order` instead of `In 
> > > > order`).
> > > > * While this is a cute way to get the point across, we want the 
> > > > diagnostic to help guide the user as to why their code is wrong so they 
> > > > have the best chance to fix the issue.
> > > > 
> > > > I think the original wording isn't too bad here because it's lexically 
> > > > associated with the function definition itself (so the "this function" 
> > > > is clear in situ), but I do think "will call itself" is a bit awkward. 
> > > > Perhaps "this function is called recursively on all paths through the 
> > > > function", but I don't feel strongly either.
> > > I like the suggestion by Aaron here, "function %0 is called recursively 
> > > through all paths through %0" (or the function).  Perhaps the double %0 
> > > is a touch of a recursion joke as well :) 
> > TBH, I'd probably skip the %0 because the warning is issued on the same 
> > line as the function definition itself, so the name should already be 
> > obvious to the user.
> > 
> > This diagnostic doesn't currently handle mutually recursive functions, and 
> > I kind of worry that adding %0 implies we support that case when we don't.
> **"this function is called recursively on all paths through the function"**  
> is good. what you say which will be more better as the  **"all paths through 
> this function will call itself"**  is also ok. 
if i am not wrong it is indicating the infinite recursion as no base case. can 
we prompt user in some a decent manner that the base case is missing. i.e. 
including some phrase in **"this function is called recursively on all paths 
through the function"** that shows base case was missing or this would be a bad 
idea? any suggestions.



Comment at: clang/test/SemaCXX/warn-infinite-recursion.cpp:11
   else
-b(x+1);
+b(x + 1);
 }

aaron.ballman wrote:
> It looks like you formatted the whole file -- we typically try to have the 
> only changes in the patch be specifically about the changes driving the 
> patch, instead of mixing several fixes together into one patch. While it 
> seems a bit less efficient than it could be, having a clean separation eases 
> maintenance burdens for us. If we need to revert a patch for some reason, we 
> don't lose potentially good changes along with the bad ones. But also, it 
> makes it more 

[clang] 6ccc273 - Update ASTImportError.h

[via cfe-commits]

Author: Shivam
Date: 2022-06-15T00:45:07+05:30
New Revision: 6ccc2733e72017999a94c10147a71ff595286080

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

LOG: Update ASTImportError.h

Added: 


Modified: 
clang/include/clang/AST/ASTImportError.h

Removed: 




diff  --git a/clang/include/clang/AST/ASTImportError.h 
b/clang/include/clang/AST/ASTImportError.h
index b33d8817e521..405790b6ded3 100644
--- a/clang/include/clang/AST/ASTImportError.h
+++ b/clang/include/clang/AST/ASTImportError.h
@@ -18,11 +18,8 @@
 
 namespace clang {
 
-<<< HEAD
 class ASTImportError : public llvm::ErrorInfo {
-===
-class ImportError : public llvm::ErrorInfo {
->>> 4bf928bce44adda059aba715664c41462536d483
+
 public:
   /// \brief Kind of error when importing an AST component.
   enum ErrorKind {
@@ -35,7 +32,6 @@ class ImportError : public llvm::ErrorInfo {
 
   static char ID;
 
-<<< HEAD
   ASTImportError() : Error(Unknown) {}
   ASTImportError(const ASTImportError ) : Error(Other.Error) {}
   ASTImportError =(const ASTImportError ) {
@@ -43,15 +39,6 @@ class ImportError : public llvm::ErrorInfo {
 return *this;
   }
   ASTImportError(ErrorKind Error) : Error(Error) {}
-===
-  ImportError() : Error(Unknown) {}
-  ImportError(const ImportError ) : Error(Other.Error) {}
-  ImportError =(const ImportError ) {
-Error = Other.Error;
-return *this;
-  }
-  ImportError(ErrorKind Error) : Error(Error) {}
->>> 4bf928bce44adda059aba715664c41462536d483
 
   std::string toString() const;
 



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

[clang] caf2767 - [Clang][AST] Fixed BindingDecl AST-dump for tuple like structures

[via cfe-commits]

Author: isuckatcs
Date: 2022-06-14T21:13:04+02:00
New Revision: caf2767c47c3808538c82b6ea2b24d8e95b11ee7

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

LOG: [Clang][AST] Fixed BindingDecl AST-dump for tuple like structures

The AST of a BindingDecl in case of tuple like structures wasn't
properly printed. For these bidnings there is information stored
in BindingDecl::getHoldingVar(), and this information was't
printed in the AST-dump.

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

Added: 


Modified: 
clang/include/clang/AST/ASTNodeTraverser.h
clang/unittests/AST/ASTTraverserTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTNodeTraverser.h 
b/clang/include/clang/AST/ASTNodeTraverser.h
index f2c5c01ac88d..093dc9a773c7 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -467,6 +467,10 @@ class ASTNodeTraverser
   void VisitBindingDecl(const BindingDecl *D) {
 if (Traversal == TK_IgnoreUnlessSpelledInSource)
   return;
+
+if (const auto *V = D->getHoldingVar())
+  Visit(V);
+
 if (const auto *E = D->getBinding())
   Visit(E);
   }

diff  --git a/clang/unittests/AST/ASTTraverserTest.cpp 
b/clang/unittests/AST/ASTTraverserTest.cpp
index 9d32a3d4116d..3553c3012cd4 100644
--- a/clang/unittests/AST/ASTTraverserTest.cpp
+++ b/clang/unittests/AST/ASTTraverserTest.cpp
@@ -1157,6 +1157,46 @@ void decomposition()
 f = 42;
 }
 
+typedef __typeof(sizeof(int)) size_t;
+
+struct Pair
+{
+int x, y;
+};
+
+// Note: these utilities are required to force binding to tuple like structure
+namespace std
+{
+template 
+struct tuple_size
+{
+};
+
+template <>
+struct tuple_size
+{
+static constexpr size_t value = 2;
+};
+
+template 
+struct tuple_element
+{
+using type = int;
+};
+
+};
+
+template 
+int &(Pair &);
+
+void decompTuple()
+{
+Pair p{1, 2};
+auto [a, b] = p;
+
+a = 3;
+}
+
 )cpp",
{"-std=c++20"});
 
@@ -1492,6 +1532,48 @@ DecompositionDecl ''
 |-BindingDecl 'f'
 |-BindingDecl 's'
 `-BindingDecl 't'
+)cpp");
+  }
+
+  {
+auto FN = ast_matchers::match(
+functionDecl(hasName("decompTuple"),
+ hasDescendant(decompositionDecl().bind("decomp"))),
+AST2->getASTContext());
+EXPECT_EQ(FN.size(), 1u);
+
+EXPECT_EQ(
+dumpASTString(TK_AsIs, FN[0].getNodeAs("decomp")),
+R"cpp(
+DecompositionDecl ''
+|-CXXConstructExpr
+| `-ImplicitCastExpr
+|   `-DeclRefExpr 'p'
+|-BindingDecl 'a'
+| |-VarDecl 'a'
+| | `-CallExpr
+| |   |-ImplicitCastExpr
+| |   | `-DeclRefExpr 'get'
+| |   `-ImplicitCastExpr
+| | `-DeclRefExpr ''
+| `-DeclRefExpr 'a'
+`-BindingDecl 'b'
+  |-VarDecl 'b'
+  | `-CallExpr
+  |   |-ImplicitCastExpr
+  |   | `-DeclRefExpr 'get'
+  |   `-ImplicitCastExpr
+  | `-DeclRefExpr ''
+  `-DeclRefExpr 'b'
+)cpp");
+
+EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource,
+FN[0].getNodeAs("decomp")),
+  R"cpp(
+DecompositionDecl ''
+|-DeclRefExpr 'p'
+|-BindingDecl 'a'
+`-BindingDecl 'b'
 )cpp");
   }
 }



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

[PATCH] D126131: [Clang][AST] BindingDecl ASTDump for tuple like structure

[Domján Dániel via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcaf2767c47c3: [Clang][AST] Fixed BindingDecl AST-dump for 
tuple like structures (authored by isuckatcs).
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126131

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/unittests/AST/ASTTraverserTest.cpp

Index: clang/unittests/AST/ASTTraverserTest.cpp
===
--- clang/unittests/AST/ASTTraverserTest.cpp
+++ clang/unittests/AST/ASTTraverserTest.cpp
@@ -1157,6 +1157,46 @@
 f = 42;
 }
 
+typedef __typeof(sizeof(int)) size_t;
+
+struct Pair
+{
+int x, y;
+};
+
+// Note: these utilities are required to force binding to tuple like structure
+namespace std
+{
+template 
+struct tuple_size
+{
+};
+
+template <>
+struct tuple_size
+{
+static constexpr size_t value = 2;
+};
+
+template 
+struct tuple_element
+{
+using type = int;
+};
+
+};
+
+template 
+int &(Pair &);
+
+void decompTuple()
+{
+Pair p{1, 2};
+auto [a, b] = p;
+
+a = 3;
+}
+
 )cpp",
{"-std=c++20"});
 
@@ -1492,6 +1532,48 @@
 |-BindingDecl 'f'
 |-BindingDecl 's'
 `-BindingDecl 't'
+)cpp");
+  }
+
+  {
+auto FN = ast_matchers::match(
+functionDecl(hasName("decompTuple"),
+ hasDescendant(decompositionDecl().bind("decomp"))),
+AST2->getASTContext());
+EXPECT_EQ(FN.size(), 1u);
+
+EXPECT_EQ(
+dumpASTString(TK_AsIs, FN[0].getNodeAs("decomp")),
+R"cpp(
+DecompositionDecl ''
+|-CXXConstructExpr
+| `-ImplicitCastExpr
+|   `-DeclRefExpr 'p'
+|-BindingDecl 'a'
+| |-VarDecl 'a'
+| | `-CallExpr
+| |   |-ImplicitCastExpr
+| |   | `-DeclRefExpr 'get'
+| |   `-ImplicitCastExpr
+| | `-DeclRefExpr ''
+| `-DeclRefExpr 'a'
+`-BindingDecl 'b'
+  |-VarDecl 'b'
+  | `-CallExpr
+  |   |-ImplicitCastExpr
+  |   | `-DeclRefExpr 'get'
+  |   `-ImplicitCastExpr
+  | `-DeclRefExpr ''
+  `-DeclRefExpr 'b'
+)cpp");
+
+EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource,
+FN[0].getNodeAs("decomp")),
+  R"cpp(
+DecompositionDecl ''
+|-DeclRefExpr 'p'
+|-BindingDecl 'a'
+`-BindingDecl 'b'
 )cpp");
   }
 }
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -467,6 +467,10 @@
   void VisitBindingDecl(const BindingDecl *D) {
 if (Traversal == TK_IgnoreUnlessSpelledInSource)
   return;
+
+if (const auto *V = D->getHoldingVar())
+  Visit(V);
+
 if (const auto *E = D->getBinding())
   Visit(E);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127762: [Clang][AArch64] Add ACLE attributes for SME.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

> This patch adds all the language-level function attributes defined in:
>
> https://github.com/ARM-software/acle/pull/188

This pull request has not yet been merged, so what are the chances that it gets 
rejected or altered?

> LLVM support for these attributes will follow later, so at the moment these 
> attributes are non-functional.

FWIW, I would rather we land the attributes once they're functional even if we 
do a bunch of the review work up front. While rare, we have run into the 
situation where attributes get added with intentions to make them useful later 
but then something comes up and we're left with half the implementation exposed 
to users.




Comment at: clang/include/clang/AST/Type.h:3806-3811
+SME_NormalFunction = 0,
+SME_PStateSMEnabledMask = 1,
+SME_PStateSMCompatibleMask = 2,
+SME_PStateZANewMask = 4,
+SME_PStateZASharedMask = 8,
+SME_PStateZAPreservedMask = 16,

Also, would it be better to use a bitmask enumeration here? 
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/BitmaskEnum.h



Comment at: clang/include/clang/AST/Type.h:4008
 bool HasTrailingReturn : 1;
+unsigned AArch64SMEAttributes : 8;
 Qualifiers TypeQuals;

So functions without prototypes cannot have any of these attributes?



Comment at: clang/include/clang/Basic/Attr.td:2322
 
+def ArmStreamingCompatible : DeclOrTypeAttr, TargetSpecificAttr 
{
+  let Spellings = [Clang<"arm_streaming_compatible">];

`DeclOrTypeAttr` is only intended to be used for attributes which were 
historically a declaration attribute but are semantically type attributes (like 
calling conventions). It seems to me that each of these is purely a type 
attribute and we shouldn't allow them to be written in the declaration 
position. WDYT?



Comment at: clang/include/clang/Basic/Attr.td:2325
+  let Subjects = SubjectList<[FunctionLike], ErrorDiag>;
+  let Documentation = [Undocumented];
+}

No new, undocumented attributes (same goes for the rest of the additions); 
please add some docs to AttrDocs.td (it's okay to lightly document and point to 
a canonical resource for more details).



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8091-8103
+  // Handle attributes that are invalid without SME
+  switch (A.getKind()) {
+  case ParsedAttr::AT_ArmStreaming:
+  case ParsedAttr::AT_ArmLocallyStreaming:
+  case ParsedAttr::AT_ArmSharedZA:
+  case ParsedAttr::AT_ArmPreservesZA:
+  case ParsedAttr::AT_ArmNewZA:

I think this should be handled declaratively in Attr.td by defining a new 
`TargetArch` that checks this property, similar to: 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/Attr.td#L387



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8109-8112
+  // Handle mutually exclusive SME function attributes:
+  //  -  arm_streaming & arm_streaming_compatible
+  //  -  arm_new_za & arm_preserves_za
+  //  -  arm_new_za & arm_shared_za

This should be handled declaratively in Attr.td, like this: 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/Attr.td#L1403



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:9078-9085
+  case ParsedAttr::AT_ArmStreaming:
+  case ParsedAttr::AT_ArmStreamingCompatible:
+  case ParsedAttr::AT_ArmLocallyStreaming:
+  case ParsedAttr::AT_ArmSharedZA:
+  case ParsedAttr::AT_ArmPreservesZA:
+  case ParsedAttr::AT_ArmNewZA:
+handleSMEAttrs(S, D, AL);

If the attributes are no longer declaration attributes, then all of this can go 
away because it needs to be handled from SemaType.cpp instead.



Comment at: clang/lib/Sema/SemaType.cpp:7669-7672
+const FunctionProtoType *FnTy = 
unwrapped.get()->getAs();
+if (!FnTy) {
+  // SME ACLE attributes are not supported on K unprototyped C
+  // functions.

Ah, I think the `Subjects` list in Attr.td is incorrect then; it seems like you 
want to use `HasFunctionProto` instead of `FunctionLike` (or something more 
similar to `HasFunctionProto` if you really don't want to support ObjC methods 
or blocks).



Comment at: clang/lib/Sema/SemaType.cpp:7676
+  return false;
+}
+

Unfortunately, type attributes do not yet have any of the automated checking 
generated by tablegen, so there are no calls to 
`Sema::checkCommonAttributeFeatures()`, which means you have to manually check 
things like mutual exclusions, not accepting arguments, etc.



Comment at: clang/test/Sema/aarch64-sme-attrs-no-sme.c:1
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu 
-fallow-half-arguments-and-returns -fsyntax-only -verify %s
+

I assume that option wasn't necessary for the test?



Comment at: 

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Shivam Rajput via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9d637956b792: [clang][NFC][AST] rename the ImportError to 
ASTImportError (authored by phyBrackets).

Changed prior to commit:
  https://reviews.llvm.org/D125340?vs=428508=436881#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

Files:
  clang/include/clang/AST/ASTImportError.h
  clang/include/clang/AST/ASTImporter.h
  clang/include/clang/AST/ASTImporterSharedState.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/unittests/AST/ASTImporterFixtures.h
  clang/unittests/AST/ASTImporterODRStrategiesTest.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -3208,8 +3208,8 @@
 
   auto ToBlockOrError = importOrError(FromBlock, Lang_CXX03);
 
-  const auto ExpectUnsupportedConstructError = [](const ImportError ) {
-EXPECT_EQ(ImportError::UnsupportedConstruct, Error.Error);
+  const auto ExpectUnsupportedConstructError = [](const ASTImportError ) {
+EXPECT_EQ(ASTImportError::UnsupportedConstruct, Error.Error);
   };
   llvm::handleAllErrors(ToBlockOrError.takeError(),
 ExpectUnsupportedConstructError);
@@ -5469,9 +5469,9 @@
 
   // But an error is set to the counterpart in the "from" context.
   ASTImporter *Importer = findFromTU(FromSpec)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromSpec);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromSpec);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::NameConflict);
+  EXPECT_EQ(OptErr->Error, ASTImportError::NameConflict);
 }
 
 // Check a case when a new AST node is created but not linked to the AST before
@@ -5493,9 +5493,9 @@
   0u);
 
   ASTImporter *Importer = findFromTU(FromFoo)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromFoo);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromFoo);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
 }
 
 // Check a case when a new AST node is created and linked to the AST before
@@ -5526,12 +5526,13 @@
   // An error is set to the counterpart in the "from" context both for the fwd
   // decl and the definition.
   ASTImporter *Importer = findFromTU(FromProto)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromProto);
+  Optional OptErr =
+  Importer->getImportDeclErrorIfAny(FromProto);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
   OptErr = Importer->getImportDeclErrorIfAny(FromDef);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
 }
 
 // An error should be set for a class if we cannot import one member.
@@ -5551,16 +5552,16 @@
   // An error is set for X.
   EXPECT_FALSE(ImportedX);
   ASTImporter *Importer = findFromTU(FromX)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromX);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromX);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
 
   // An error is set for f().
   auto *FromF = FirstDeclMatcher().match(
   FromTU, cxxMethodDecl(hasName("f")));
   OptErr = Importer->getImportDeclErrorIfAny(FromF);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
   // And any subsequent import should fail.
   CXXMethodDecl *ImportedF = Import(FromF, Lang_CXX03);
   EXPECT_FALSE(ImportedF);
@@ -5618,7 +5619,7 @@
 
   // An error is set to the templated CXXRecordDecl of F.
   ASTImporter *Importer = findFromTU(FromFRD)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromFRD);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromFRD);
   EXPECT_TRUE(OptErr);
 
   // An error is set to A.
@@ -5676,7 +5677,7 @@
   // There is no error set for X.
   EXPECT_TRUE(ImportedX);
   ASTImporter *Importer = findFromTU(FromX)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromX);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromX);
   ASSERT_FALSE(OptErr);
 
   // An error is set for f().
@@ -5684,7 +5685,7 @@
   FromTU, functionDecl(hasName("f")));
   OptErr = Importer->getImportDeclErrorIfAny(FromF);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  

[clang] 9d63795 - [clang][NFC][AST] rename the ImportError to ASTImportError

[via cfe-commits]

Author: phyBrackets
Date: 2022-06-15T00:40:32+05:30
New Revision: 9d637956b79282aa0980e859b9128cf09a83dda6

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

LOG: [clang][NFC][AST] rename the ImportError to ASTImportError

this patch is the continuation of my previous patch regarding the ImportError 
in ASTImportError.h

Reviewed By: martong

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

Added: 


Modified: 
clang/include/clang/AST/ASTImportError.h
clang/include/clang/AST/ASTImporter.h
clang/include/clang/AST/ASTImporterSharedState.h
clang/lib/AST/ASTImporter.cpp
clang/lib/CrossTU/CrossTranslationUnit.cpp
clang/unittests/AST/ASTImporterFixtures.h
clang/unittests/AST/ASTImporterODRStrategiesTest.cpp
clang/unittests/AST/ASTImporterTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTImportError.h 
b/clang/include/clang/AST/ASTImportError.h
index 034bd50cc7e15..b33d8817e5214 100644
--- a/clang/include/clang/AST/ASTImportError.h
+++ b/clang/include/clang/AST/ASTImportError.h
@@ -18,7 +18,11 @@
 
 namespace clang {
 
+<<< HEAD
+class ASTImportError : public llvm::ErrorInfo {
+===
 class ImportError : public llvm::ErrorInfo {
+>>> 4bf928bce44adda059aba715664c41462536d483
 public:
   /// \brief Kind of error when importing an AST component.
   enum ErrorKind {
@@ -31,6 +35,15 @@ class ImportError : public llvm::ErrorInfo {
 
   static char ID;
 
+<<< HEAD
+  ASTImportError() : Error(Unknown) {}
+  ASTImportError(const ASTImportError ) : Error(Other.Error) {}
+  ASTImportError =(const ASTImportError ) {
+Error = Other.Error;
+return *this;
+  }
+  ASTImportError(ErrorKind Error) : Error(Error) {}
+===
   ImportError() : Error(Unknown) {}
   ImportError(const ImportError ) : Error(Other.Error) {}
   ImportError =(const ImportError ) {
@@ -38,6 +51,7 @@ class ImportError : public llvm::ErrorInfo {
 return *this;
   }
   ImportError(ErrorKind Error) : Error(Error) {}
+>>> 4bf928bce44adda059aba715664c41462536d483
 
   std::string toString() const;
 

diff  --git a/clang/include/clang/AST/ASTImporter.h 
b/clang/include/clang/AST/ASTImporter.h
index 7d413115d769c..e38ffa50ad4ab 100644
--- a/clang/include/clang/AST/ASTImporter.h
+++ b/clang/include/clang/AST/ASTImporter.h
@@ -231,7 +231,7 @@ class TypeSourceInfo;
 /// imported. The same declaration may or may not be included in
 /// ImportedDecls. This map is updated continuously during imports and 
never
 /// cleared (like ImportedDecls).
-llvm::DenseMap ImportDeclErrors;
+llvm::DenseMap ImportDeclErrors;
 
 /// Mapping from the already-imported declarations in the "to"
 /// context to the corresponding declarations in the "from" context.
@@ -564,10 +564,10 @@ class TypeSourceInfo;
 /// Return if import of the given declaration has failed and if yes
 /// the kind of the problem. This gives the first error encountered with
 /// the node.
-llvm::Optional getImportDeclErrorIfAny(Decl *FromD) const;
+llvm::Optional getImportDeclErrorIfAny(Decl *FromD) const;
 
 /// Mark (newly) imported declaration with error.
-void setImportDeclError(Decl *From, ImportError Error);
+void setImportDeclError(Decl *From, ASTImportError Error);
 
 /// Determine whether the given types are structurally
 /// equivalent.

diff  --git a/clang/include/clang/AST/ASTImporterSharedState.h 
b/clang/include/clang/AST/ASTImporterSharedState.h
index a64c9d6be455f..ce7fb290c9765 100644
--- a/clang/include/clang/AST/ASTImporterSharedState.h
+++ b/clang/include/clang/AST/ASTImporterSharedState.h
@@ -36,7 +36,7 @@ class ASTImporterSharedState {
   /// imported. The same declaration may or may not be included in
   /// ImportedFromDecls. This map is updated continuously during imports and
   /// never cleared (like ImportedFromDecls).
-  llvm::DenseMap ImportErrors;
+  llvm::DenseMap ImportErrors;
 
   /// Set of the newly created declarations.
   llvm::DenseSet NewDecls;
@@ -65,15 +65,15 @@ class ASTImporterSharedState {
 LookupTable->remove(ND);
   }
 
-  llvm::Optional getImportDeclErrorIfAny(Decl *ToD) const {
+  llvm::Optional getImportDeclErrorIfAny(Decl *ToD) const {
 auto Pos = ImportErrors.find(ToD);
 if (Pos != ImportErrors.end())
   return Pos->second;
 else
-  return Optional();
+  return Optional();
   }
 
-  void setImportDeclError(Decl *To, ImportError Error) {
+  void setImportDeclError(Decl *To, ASTImportError Error) {
 ImportErrors[To] = Error;
   }
 

diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 7fee8d21fa39e..8d930ead97d3e 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -84,7 +84,7 @@ namespace clang {
   

[PATCH] D127460: Rename GCCBuiltin into ClangBuiltin

[Guillaume Gomez via Phabricator via cfe-commits]

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

Fixed @spatel's comment.




Comment at: llvm/include/llvm/IR/Intrinsics.td:387
 
-/// GCCBuiltin - If this intrinsic exactly corresponds to a GCC builtin, this
+/// ClangBuiltin - If this intrinsic exactly corresponds to a GCC builtin, this
 /// specifies the name of the builtin.  This provides automatic CBE and CFE

spatel wrote:
> Replace "GCC" with "Clang" ?
Good catch, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127460

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

[PATCH] D127460: Rename GCCBuiltin into ClangBuiltin

[Guillaume Gomez via Phabricator via cfe-commits]

GuillaumeGomez updated this revision to Diff 436876.
GuillaumeGomez added a comment.

- Update #define name as well
- Rename `IsGCC` function parameter into `IsClang`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127460

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/IR/Intrinsics.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/include/llvm/IR/IntrinsicsAMDGPU.td
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/include/llvm/IR/IntrinsicsBPF.td
  llvm/include/llvm/IR/IntrinsicsHexagon.td
  llvm/include/llvm/IR/IntrinsicsMips.td
  llvm/include/llvm/IR/IntrinsicsNVVM.td
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/include/llvm/IR/IntrinsicsSystemZ.td
  llvm/include/llvm/IR/IntrinsicsVE.td
  llvm/include/llvm/IR/IntrinsicsVEVL.gen.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/IR/IntrinsicsXCore.td
  llvm/lib/IR/Function.cpp
  llvm/utils/TableGen/CodeGenIntrinsics.h
  llvm/utils/TableGen/CodeGenTarget.cpp
  llvm/utils/TableGen/IntrinsicEmitter.cpp

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

[PATCH] D127528: [Clang] Let the linker choose shared or static libunwind unless specified

[Stephen Hines via Phabricator via cfe-commits]

srhines added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1477
   if (Args.hasArg(options::OPT_shared_libgcc))
 return LibGccType::SharedLibGcc;
-  // The Android NDK only provides libunwind.a, not libunwind.so.

phosek wrote:
> mstorsjo wrote:
> > Previously, `OPT_shared_libgcc` would override the default for android, but 
> > now it no longer would. I guess that might be sensible, but it seems 
> > slightly outside of what the patch says it does. (I guess it's fine to 
> > include the change too, but I just wanted to raise it for discussion.)
> I was going by the comment, but I'm fine preserving the existing behavior. 
> @srhines do you know if `-shared-libgcc` should be supported on Android?
No, there's no such shared library to even depend on, so it wouldn't be 
supported previously or in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127528

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

[PATCH] D127460: Rename GCCBuiltin into ClangBuiltin

[Sanjay Patel via Phabricator via cfe-commits]

spatel added inline comments.



Comment at: llvm/include/llvm/IR/Intrinsics.td:387
 
-/// GCCBuiltin - If this intrinsic exactly corresponds to a GCC builtin, this
+/// ClangBuiltin - If this intrinsic exactly corresponds to a GCC builtin, this
 /// specifies the name of the builtin.  This provides automatic CBE and CFE

Replace "GCC" with "Clang" ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127460

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[John Ericson via Phabricator via cfe-commits]

Ericson2314 updated this revision to Diff 436874.
Ericson2314 added a comment.

Rebase again, to try to figure out if last round of errors were spurious


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

Files:
  bolt/tools/CMakeLists.txt
  bolt/tools/driver/CMakeLists.txt
  bolt/tools/heatmap/CMakeLists.txt
  bolt/tools/merge-fdata/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/cmake/modules/TableGen.cmake
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  mlir/tools/mlir-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-linalg-ods-gen/CMakeLists.txt
  mlir/tools/mlir-lsp-server/CMakeLists.txt
  mlir/tools/mlir-opt/CMakeLists.txt
  mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
  mlir/tools/mlir-reduce/CMakeLists.txt
  mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
  mlir/tools/mlir-translate/CMakeLists.txt
  mlir/tools/mlir-vulkan-runner/CMakeLists.txt
  mlir/tools/tblgen-lsp-server/CMakeLists.txt
  openmp/libomptarget/tools/CMakeLists.txt
  openmp/libomptarget/tools/deviceinfo/CMakeLists.txt

Index: openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
===
--- openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
+++ openmp/libomptarget/tools/deviceinfo/CMakeLists.txt
@@ -12,7 +12,7 @@
 
 libomptarget_say("Building the llvm-omp-device-info tool")
 
-add_llvm_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
+add_openmp_tool(llvm-omp-device-info llvm-omp-device-info.cpp)
 
 llvm_update_compile_flags(llvm-omp-device-info)
 
Index: openmp/libomptarget/tools/CMakeLists.txt
===
--- openmp/libomptarget/tools/CMakeLists.txt
+++ openmp/libomptarget/tools/CMakeLists.txt
@@ -10,4 +10,18 @@
 #
 ##===--===##
 
+set(OPENMP_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH
+"Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
+mark_as_advanced(OPENMP_TOOLS_INSTALL_DIR)
+
+# Move these macros to AddOpenMP if such a CMake module is ever created.
+
+macro(add_openmp_tool name)
+  llvm_add_tool(OPENMP ${ARGV})
+endmacro()
+
+macro(add_openmp_tool_symlink name)
+  llvm_add_tool_symlink(OPENMP ${ARGV})
+endmacro()
+
 add_subdirectory(deviceinfo)
Index: mlir/tools/tblgen-lsp-server/CMakeLists.txt
===
--- mlir/tools/tblgen-lsp-server/CMakeLists.txt
+++ mlir/tools/tblgen-lsp-server/CMakeLists.txt
@@ -2,7 +2,7 @@
   TableGenLspServerLib
   )
 
-add_llvm_tool(tblgen-lsp-server
+add_mlir_tool(tblgen-lsp-server
   tblgen-lsp-server.cpp
 
   DEPENDS
Index: mlir/tools/mlir-vulkan-runner/CMakeLists.txt
===
--- mlir/tools/mlir-vulkan-runner/CMakeLists.txt
+++ mlir/tools/mlir-vulkan-runner/CMakeLists.txt
@@ -88,7 +88,7 @@
 LIST(APPEND targets_to_link "LLVM${t}")
   ENDFOREACH(t)
 
-  add_llvm_tool(mlir-vulkan-runner
+  add_mlir_tool(mlir-vulkan-runner
 mlir-vulkan-runner.cpp
 
 DEPENDS
Index: mlir/tools/mlir-translate/CMakeLists.txt
===
--- mlir/tools/mlir-translate/CMakeLists.txt
+++ mlir/tools/mlir-translate/CMakeLists.txt
@@ -5,7 +5,7 @@
 get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
 get_property(translation_libs GLOBAL PROPERTY MLIR_TRANSLATION_LIBS)
 
-add_llvm_tool(mlir-translate
+add_mlir_tool(mlir-translate
   mlir-translate.cpp
   )
 llvm_update_compile_flags(mlir-translate)
Index: mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
===
--- mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
+++ mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
@@ -5,7 +5,7 @@
 if (MLIR_ENABLE_SPIRV_CPU_RUNNER)
   message(STATUS "Building SPIR-V CPU runner")
 
-  add_llvm_tool(mlir-spirv-cpu-runner
+  add_mlir_tool(mlir-spirv-cpu-runner
 mlir-spirv-cpu-runner.cpp
   )
 
Index: mlir/tools/mlir-reduce/CMakeLists.txt
===
--- mlir/tools/mlir-reduce/CMakeLists.txt
+++ mlir/tools/mlir-reduce/CMakeLists.txt
@@ -17,7 +17,7 @@
   MLIRReduceLib
   )
 
-add_llvm_tool(mlir-reduce
+add_mlir_tool(mlir-reduce
   mlir-reduce.cpp
 
   DEPENDS
Index: mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
===
--- mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
+++ mlir/tools/mlir-pdll-lsp-server/CMakeLists.txt
@@ -2,7 +2,7 @@
   MLIRPdllLspServerLib
   )
 
-add_llvm_tool(mlir-pdll-lsp-server

[PATCH] D127771: [HIP] fix long double size

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D127771#3582640 , @tra wrote:

> AFAICT, the test case you've added works fine with the compiler at HEAD: 
> https://cuda.godbolt.org/z/q3xYMfdeb
> I guess it only shows up in assertion-enabled builds. Can you check what 
> happens if you run the test case compiled as CUDA? I suspect it will have the 
> same issue -- we don't override setAuxTarget at all.

The assertion only happens with assertion-enabled builds. However, the test is 
able to detect the issue with assertion-disabled builds since it checks the 
sizeof(long double). In the old build it is 16.

The lit test contains run line for nvptx. It seems nvptx is not subject to this 
issue since sizeof(long double)==8.


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

https://reviews.llvm.org/D127771

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

[PATCH] D126984: [clang] Add support for optimize function attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Also note that there is a '#pragma GCC optimize' pragma. After this patch, it 
should be hard to implement it.

https://gcc.gnu.org/onlinedocs/gcc/Function-Specific-Option-Pragmas.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

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

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

[Javier Alvarez via Phabricator via cfe-commits]

Javier-varez added a comment.

Thanks again for having another look at the changes! I do not have commit 
access since this is my first change to LLVM. I would appreciate if you could 
merge it. here's my username and email:
Javier Alvarez 

And let me know if there's anything else you'd like to modify :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

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

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

[Javier Alvarez via Phabricator via cfe-commits]

Javier-varez updated this revision to Diff 436871.
Javier-varez marked 2 inline comments as done.
Javier-varez added a comment.

Address review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

Files:
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/drs/dr21xx.cpp
  clang/test/CXX/special/class.copy/p12-0x.cpp
  clang/test/CXX/special/class.copy/p25-0x.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -12840,7 +12840,7 @@
 https://wg21.link/cwg2171;>2171
 CD4
 Triviality of copy constructor with less-qualified parameter
-Unknown
+Clang 15
   
   
 https://wg21.link/cwg2172;>2172
Index: clang/test/CXX/special/class.copy/p25-0x.cpp
===
--- clang/test/CXX/special/class.copy/p25-0x.cpp
+++ clang/test/CXX/special/class.copy/p25-0x.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
 
 // expected-no-diagnostics
 
@@ -31,7 +32,30 @@
 struct NonConstCopy {
   NonConstCopy =(NonConstCopy &) = default;
 };
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 14
+// Up until (and including) Clang 14, non-const copy assignment operators were not trivial because
+// of dr2171
 using _ = not_trivially_assignable;
+#else
+// In the latest Clang version, all defaulted assignment operators are trivial, even if non-const,
+// because dr2171 is fixed
+static_assert(__has_trivial_assign(NonConstCopy), "");
+static_assert(__is_trivially_assignable(NonConstCopy &, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy &&), "");
+static_assert(__is_trivially_assignable(NonConstCopy &&, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers =(const DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers =(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers =(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_assignable;
+#endif
 
 // class X has no virtual functions
 struct VFn {
Index: clang/test/CXX/special/class.copy/p12-0x.cpp
===
--- clang/test/CXX/special/class.copy/p12-0x.cpp
+++ clang/test/CXX/special/class.copy/p12-0x.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted
+// RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
 
 // expected-no-diagnostics
 
@@ -28,7 +29,25 @@
 struct NonConstCopy {
   NonConstCopy(NonConstCopy &) = default;
 };
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 14
+// Up until (and including) Clang 14, non-const copy constructors were not trivial because of dr2171
 using _ = not_trivially_copyable;
+#else
+// In the latest Clang version, all defaulted constructors are trivial, even if non-const, because
+// dr2171 is fixed.
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, const NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers(const DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_copyable;
+#endif
 
 // class X has no virtual functions
 struct VFn {
Index: clang/test/CXX/drs/dr21xx.cpp
===
--- clang/test/CXX/drs/dr21xx.cpp
+++ clang/test/CXX/drs/dr21xx.cpp
@@ -140,6 +140,33 @@
 #endif
 }
 
+namespace dr2171 { // dr2171: 15
+#if __cplusplus >= 201103L
+
+struct NonConstCopy {
+  NonConstCopy(NonConstCopy &) = default;
+  NonConstCopy =(NonConstCopy &) = default;
+};
+
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy 

[PATCH] D127771: [HIP] fix long double size

[Artem Belevich via Phabricator via cfe-commits]

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

AFAICT, the test case you've added works fine with the compiler at HEAD: 
https://cuda.godbolt.org/z/q3xYMfdeb
I guess it only shows up in assertion-enabled builds. Can you check what 
happens if you run the test case compiled as CUDA? I suspect it will have the 
same issue -- we don't override setAuxTarget at all.


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

https://reviews.llvm.org/D127771

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

[PATCH] D127528: [Clang] Let the linker choose shared or static libunwind unless specified

[Petr Hosek via Phabricator via cfe-commits]

phosek added a subscriber: srhines.
phosek added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1477
   if (Args.hasArg(options::OPT_shared_libgcc))
 return LibGccType::SharedLibGcc;
-  // The Android NDK only provides libunwind.a, not libunwind.so.

mstorsjo wrote:
> Previously, `OPT_shared_libgcc` would override the default for android, but 
> now it no longer would. I guess that might be sensible, but it seems slightly 
> outside of what the patch says it does. (I guess it's fine to include the 
> change too, but I just wanted to raise it for discussion.)
I was going by the comment, but I'm fine preserving the existing behavior. 
@srhines do you know if `-shared-libgcc` should be supported on Android?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127528

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

[PATCH] D127641: [clang-cl][MSVC] Enable /Zc:alignedNew for C++17 and /Zc:sizedDealloc by default

[Stephen Long via Phabricator via cfe-commits]

steplong added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6641
   // by default.
   if (Arg *A = Args.getLastArg(options::OPT_faligned_allocation,
options::OPT_fno_aligned_allocation,

There's a check here for faligned-allocation, but I wasn't sure how to get the 
c++ standard here. Also, I couldn't figure a clean way to compare if the 
standard was c++17 onwards, so I just enabled -faligned-allocation for C++17 
above


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127641

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

[PATCH] D127641: [clang-cl][MSVC] Add default /Zc conformance arguments

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 436860.
steplong added a comment.
Herald added a subscriber: MaskRay.

- Only enable /Zc:alignedNew and /Zc:sizedDealloc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127641

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cl-zc.cpp


Index: clang/test/Driver/cl-zc.cpp
===
--- clang/test/Driver/cl-zc.cpp
+++ clang/test/Driver/cl-zc.cpp
@@ -1,6 +1,12 @@
 // Note: %s must be preceded by --, otherwise it may be interpreted as a
 // command-line option, e.g. on Mac where %s is commonly under /Users.
 
+// RUN: %clang_cl /c /std:c++17 -### -- %s 2>&1 | FileCheck 
-check-prefix=ALIGNED-NEW-CPP17-DEFAULT %s
+// ALIGNED-NEW-CPP17-DEFAULT: "-faligned-allocation"
+
+// RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck 
-check-prefix=SIZED-DEALLOC-DEFAULT %s
+// SIZED-DEALLOC-DEFAULT: "-fsized-deallocation"
+
 // RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck 
-check-prefix=TRIGRAPHS-DEFAULT %s
 // cc1 will disable trigraphs for -fms-compatibility as long as -ftrigraphs
 // isn't explicitly passed.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6490,6 +6490,12 @@
 }
 
 CmdArgs.push_back(LanguageStandard.data());
+
+// MSVC enables /Zc:alignedNew by default for C++17
+if (!Args.getLastArg(options::OPT_faligned_allocation,
+ options::OPT_fno_aligned_allocation) &&
+LanguageStandard == "-std=c++17")
+  CmdArgs.push_back("-faligned-allocation");
   }
 
   Args.addOptInFlag(CmdArgs, options::OPT_fborland_extensions,
@@ -6618,6 +6624,13 @@
   Args.addOptInFlag(CmdArgs, options::OPT_frelaxed_template_template_args,
 options::OPT_fno_relaxed_template_template_args);
 
+  // MSVC turns on /Zc:sizedDealloc by default. Honor whatever is passed
+  // on the cmdline.
+  if (IsWindowsMSVC &&
+  !Args.getLastArg(options::OPT_fsized_deallocation,
+   options::OPT_fno_sized_deallocation))
+CmdArgs.push_back("-fsized-deallocation");
+
   // -fsized-deallocation is off by default, as it is an ABI-breaking change 
for
   // most platforms.
   Args.addOptInFlag(CmdArgs, options::OPT_fsized_deallocation,


Index: clang/test/Driver/cl-zc.cpp
===
--- clang/test/Driver/cl-zc.cpp
+++ clang/test/Driver/cl-zc.cpp
@@ -1,6 +1,12 @@
 // Note: %s must be preceded by --, otherwise it may be interpreted as a
 // command-line option, e.g. on Mac where %s is commonly under /Users.
 
+// RUN: %clang_cl /c /std:c++17 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-CPP17-DEFAULT %s
+// ALIGNED-NEW-CPP17-DEFAULT: "-faligned-allocation"
+
+// RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck -check-prefix=SIZED-DEALLOC-DEFAULT %s
+// SIZED-DEALLOC-DEFAULT: "-fsized-deallocation"
+
 // RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck -check-prefix=TRIGRAPHS-DEFAULT %s
 // cc1 will disable trigraphs for -fms-compatibility as long as -ftrigraphs
 // isn't explicitly passed.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6490,6 +6490,12 @@
 }
 
 CmdArgs.push_back(LanguageStandard.data());
+
+// MSVC enables /Zc:alignedNew by default for C++17
+if (!Args.getLastArg(options::OPT_faligned_allocation,
+ options::OPT_fno_aligned_allocation) &&
+LanguageStandard == "-std=c++17")
+  CmdArgs.push_back("-faligned-allocation");
   }
 
   Args.addOptInFlag(CmdArgs, options::OPT_fborland_extensions,
@@ -6618,6 +6624,13 @@
   Args.addOptInFlag(CmdArgs, options::OPT_frelaxed_template_template_args,
 options::OPT_fno_relaxed_template_template_args);
 
+  // MSVC turns on /Zc:sizedDealloc by default. Honor whatever is passed
+  // on the cmdline.
+  if (IsWindowsMSVC &&
+  !Args.getLastArg(options::OPT_fsized_deallocation,
+   options::OPT_fno_sized_deallocation))
+CmdArgs.push_back("-fsized-deallocation");
+
   // -fsized-deallocation is off by default, as it is an ABI-breaking change for
   // most platforms.
   Args.addOptInFlag(CmdArgs, options::OPT_fsized_deallocation,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126984: [clang] Add support for optimize function attribute

[Stephen Long via Phabricator via cfe-commits]

steplong added a comment.

> I appreciate your patience while we figure out the right approach here; we 
> don't usually have this many false starts when working through a feature 
> review. :-)

No worries, I appreciate the community doing the due diligence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

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

[PATCH] D127771: [HIP] fix long double size

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added subscribers: kosarev, kerbowa, tpr, jvesely.
Herald added a project: All.
yaxunl requested review of this revision.

For amdgpu target long double type is the same as double type.
The width and align of long double type was incorrectly
overridden when copying aux target properties, which
caused assertion in codegen when emitting global
variables with long double type.

This patch fix that by saving and restoring width
and align of long double type.


https://reviews.llvm.org/D127771

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/CodeGenCUDA/long-double.cu


Index: clang/test/CodeGenCUDA/long-double.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/long-double.cu
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx906 \
+// RUN:   -aux-triple x86_64-unknown-gnu-linux -fcuda-is-device \
+// RUN:   -emit-llvm -o - -x hip %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -triple nvptx \
+// RUN:   -aux-triple x86_64-unknown-gnu-linux -fcuda-is-device \
+// RUN:   -emit-llvm -o - %s 2>&1 | FileCheck %s
+
+// CHECK: @_ZN15infinity_helperIeE5valueE = {{.*}} double 0x47EFD586B834, 
align 8
+// CHECK: @size = {{.*}} i32 8
+
+#include "Inputs/cuda.h"
+
+template  struct infinity_helper {}; 
+template <> struct infinity_helper { static constexpr long double 
value = 3.4028234e38L; };
+constexpr long double infinity_helper::value;
+__device__ int size = sizeof(long double);
Index: clang/lib/Basic/Targets/AMDGPU.cpp
===
--- clang/lib/Basic/Targets/AMDGPU.cpp
+++ clang/lib/Basic/Targets/AMDGPU.cpp
@@ -462,9 +462,13 @@
   // supported by AMDGPU. Therefore keep its own format for these two types.
   auto SaveLongDoubleFormat = LongDoubleFormat;
   auto SaveFloat128Format = Float128Format;
+  auto SaveLongDoubleWidth = LongDoubleWidth;
+  auto SaveLongDoubleAlign = LongDoubleAlign;
   copyAuxTarget(Aux);
   LongDoubleFormat = SaveLongDoubleFormat;
   Float128Format = SaveFloat128Format;
+  LongDoubleWidth = SaveLongDoubleWidth;
+  LongDoubleAlign = SaveLongDoubleAlign;
   // For certain builtin types support on the host target, claim they are
   // support to pass the compilation of the host code during the device-side
   // compilation.


Index: clang/test/CodeGenCUDA/long-double.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/long-double.cu
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx906 \
+// RUN:   -aux-triple x86_64-unknown-gnu-linux -fcuda-is-device \
+// RUN:   -emit-llvm -o - -x hip %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -triple nvptx \
+// RUN:   -aux-triple x86_64-unknown-gnu-linux -fcuda-is-device \
+// RUN:   -emit-llvm -o - %s 2>&1 | FileCheck %s
+
+// CHECK: @_ZN15infinity_helperIeE5valueE = {{.*}} double 0x47EFD586B834, align 8
+// CHECK: @size = {{.*}} i32 8
+
+#include "Inputs/cuda.h"
+
+template  struct infinity_helper {}; 
+template <> struct infinity_helper { static constexpr long double value = 3.4028234e38L; };
+constexpr long double infinity_helper::value;
+__device__ int size = sizeof(long double);
Index: clang/lib/Basic/Targets/AMDGPU.cpp
===
--- clang/lib/Basic/Targets/AMDGPU.cpp
+++ clang/lib/Basic/Targets/AMDGPU.cpp
@@ -462,9 +462,13 @@
   // supported by AMDGPU. Therefore keep its own format for these two types.
   auto SaveLongDoubleFormat = LongDoubleFormat;
   auto SaveFloat128Format = Float128Format;
+  auto SaveLongDoubleWidth = LongDoubleWidth;
+  auto SaveLongDoubleAlign = LongDoubleAlign;
   copyAuxTarget(Aux);
   LongDoubleFormat = SaveLongDoubleFormat;
   Float128Format = SaveFloat128Format;
+  LongDoubleWidth = SaveLongDoubleWidth;
+  LongDoubleAlign = SaveLongDoubleAlign;
   // For certain builtin types support on the host target, claim they are
   // support to pass the compilation of the host code during the device-side
   // compilation.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126308: cmake: use llvm dir variables for clang/utils/hmaptool

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

In D126308#3582337 , @mizvekov wrote:

> In D126308#3581240 , @dyung wrote:
>
>> Can you take a look?
>
> Hi @dyung , thanks for the report and sorry for the trouble!
> Can you help me try to reproduce it?
>
> Which version of Visual Studio, and what preset are you using?

Sure, we are using Visual Studio 2019. The cmake command I use is:

  cmake.exe -G "Visual Studio 16 2019" -DCLANG_ENABLE_ARCMT=OFF 
-DCMAKE_BUILD_TYPES=Release -DLLVM_BUILD_RUNTIME=OFF 
-DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-ps4-scei -DLLVM_ENABLE_TIMESTAMPS=OFF 
-DLLVM_INCLUDE_EXAMPLES=OFF -DLLVM_TARGETS_TO_BUILD=X86 
-DLLVM_TOOL_LLD_BUILD=OFF -DLLVM_VERSION_SUFFIX= -DLLVM_LIT_ARGS="--verbose 
-j24" -Thost=x64 -DLLVM_ENABLE_PROJECTS=clang 

From the files that are generated, we then do a Release build and try to run 
the tests using that. (Although I believe any build type should be affected 
similarly.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126308

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

[PATCH] D127182: [Clang][Modules] Merge availability attributes on imported decls

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Should we add a FIXME saying we need to handle other kinds of attributes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127182

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

[PATCH] D127182: [Clang][Modules] Merge availability attributes on imported decls

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127182

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

[PATCH] D127759: [Diagnostic] Clarify -Winfinite-recursion message

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:63
 def warn_infinite_recursive_function : Warning<
-  "all paths through this function will call itself">,
+  "In order to understand recursion, you must first understand recursion">,
   InGroup, DefaultIgnore;

erichkeane wrote:
> aaron.ballman wrote:
> > A few things:
> > 
> > * Diagnostics in Clang are not intended to be grammatically correct, so 
> > they don't start with a capital letter or end with a trailing full stop (so 
> > if this wording was kept, you should use `in order` instead of `In order`).
> > * While this is a cute way to get the point across, we want the diagnostic 
> > to help guide the user as to why their code is wrong so they have the best 
> > chance to fix the issue.
> > 
> > I think the original wording isn't too bad here because it's lexically 
> > associated with the function definition itself (so the "this function" is 
> > clear in situ), but I do think "will call itself" is a bit awkward. Perhaps 
> > "this function is called recursively on all paths through the function", 
> > but I don't feel strongly either.
> I like the suggestion by Aaron here, "function %0 is called recursively 
> through all paths through %0" (or the function).  Perhaps the double %0 is a 
> touch of a recursion joke as well :) 
TBH, I'd probably skip the %0 because the warning is issued on the same line as 
the function definition itself, so the name should already be obvious to the 
user.

This diagnostic doesn't currently handle mutually recursive functions, and I 
kind of worry that adding %0 implies we support that case when we don't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127759

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

[PATCH] D126984: [clang] Add support for optimize function attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D126984#3582508 , @steplong wrote:

> I'm open to tabling this and just implementing support for an empty 
> optimization list for the pragma (i.e. `#pragma optimize("", on | off)`). For 
> our use case, at the moment, we only see the pragma being used this way. `on` 
> would be a noop (honor command-line) and `off` would mean add optnone to the 
> functions below the pragma

If that meets your needs, that is certainly a way to get you unblocked. I'm 
fine with that approach if you are.

I appreciate your patience while we figure out the right approach here; we 
don't usually have this many false starts when working through a feature 
review. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

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

[PATCH] D127759: [Diagnostic] Clarify -Winfinite-recursion message

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:63
 def warn_infinite_recursive_function : Warning<
-  "all paths through this function will call itself">,
+  "In order to understand recursion, you must first understand recursion">,
   InGroup, DefaultIgnore;

aaron.ballman wrote:
> A few things:
> 
> * Diagnostics in Clang are not intended to be grammatically correct, so they 
> don't start with a capital letter or end with a trailing full stop (so if 
> this wording was kept, you should use `in order` instead of `In order`).
> * While this is a cute way to get the point across, we want the diagnostic to 
> help guide the user as to why their code is wrong so they have the best 
> chance to fix the issue.
> 
> I think the original wording isn't too bad here because it's lexically 
> associated with the function definition itself (so the "this function" is 
> clear in situ), but I do think "will call itself" is a bit awkward. Perhaps 
> "this function is called recursively on all paths through the function", but 
> I don't feel strongly either.
I like the suggestion by Aaron here, "function %0 is called recursively through 
all paths through %0" (or the function).  Perhaps the double %0 is a touch of a 
recursion joke as well :) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127759

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

[PATCH] D127759: [Diagnostic] Clarify -Winfinite-recursion message

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: aaron.ballman, erichkeane, xgupta.
aaron.ballman added a comment.

Thank you for looking into this! I'm adding a few more reviewers for awareness.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:63
 def warn_infinite_recursive_function : Warning<
-  "all paths through this function will call itself">,
+  "In order to understand recursion, you must first understand recursion">,
   InGroup, DefaultIgnore;

A few things:

* Diagnostics in Clang are not intended to be grammatically correct, so they 
don't start with a capital letter or end with a trailing full stop (so if this 
wording was kept, you should use `in order` instead of `In order`).
* While this is a cute way to get the point across, we want the diagnostic to 
help guide the user as to why their code is wrong so they have the best chance 
to fix the issue.

I think the original wording isn't too bad here because it's lexically 
associated with the function definition itself (so the "this function" is clear 
in situ), but I do think "will call itself" is a bit awkward. Perhaps "this 
function is called recursively on all paths through the function", but I don't 
feel strongly either.



Comment at: clang/test/SemaCXX/warn-infinite-recursion.cpp:11
   else
-b(x+1);
+b(x + 1);
 }

It looks like you formatted the whole file -- we typically try to have the only 
changes in the patch be specifically about the changes driving the patch, 
instead of mixing several fixes together into one patch. While it seems a bit 
less efficient than it could be, having a clean separation eases maintenance 
burdens for us. If we need to revert a patch for some reason, we don't lose 
potentially good changes along with the bad ones. But also, it makes it more 
clear if we need to do a git-blame as to why changes were introduced.

You should revert the formatting-specific changes in the file so that the only 
differences are the functional ones.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127759

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

[PATCH] D126984: [clang] Add support for optimize function attribute

[Stephen Long via Phabricator via cfe-commits]

steplong added a comment.

I'm open to tabling this and just implementing support for an empty 
optimization list for the pragma (i.e. `#pragma optimize("", on | off)`). For 
our use case, at the moment, we only see the pragma being used this way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

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

[PATCH] D127306: [analyzer] Treat system globals as mutable if they are not const

[Balázs Benics via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:978
+assert(!Ty.isNull());
+if (Ty.isConstQualified() && Ty->isArithmeticType()) {
   // TODO: We could walk the complex types here and see if everything is

xazax.hun wrote:
> steakhal wrote:
> > xazax.hun wrote:
> > > Why do we need this extra condition? I see that this was the original 
> > > behavior, just wondering if we actually know. I think there are more 
> > > types than arithmetic that can be stored in the read only memory.
> > Well, seems reasonable. Do you think I should remove it here or in a 
> > separate patch?
> Let's keep it separate, so it can be reverted individually. Just in case we 
> find out the reason the hard way :)
D127763.



Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:979-980
+if (Ty.isConstQualified() && Ty->isArithmeticType()) {
   // TODO: We could walk the complex types here and see if everything is
   // constified.
+  sReg = getGlobalsRegion(MemRegion::GlobalImmutableSpaceRegionKind);

steakhal wrote:
> martong wrote:
> > Is this comment still meaningful? I don't think so because if the type is 
> > const, then it does not matter what sub-types it has.
> Makes sense. However, I'm not sure. Is it the case @xazax.hun?
D127763


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127306

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

[clang] 21ff652 - [analyzer][NFC] Replace getLastArg with hasArg when applicable

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-06-14T19:28:44+02:00
New Revision: 21ff652de9fc9fe7338a83e2e405aa6c5d824479

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

LOG: [analyzer][NFC] Replace getLastArg with hasArg when applicable

Depends on D126215.

Added: 


Modified: 
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index ba06b1795bf09..5d7916ace87e8 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -903,10 +903,10 @@ static bool ParseAnalyzerArgs(AnalyzerOptions , 
ArgList ,
 #include "clang/Driver/Options.inc"
 #undef ANALYZER_OPTION_WITH_MARSHALLING
 
-  if (Arg *A = Args.getLastArg(OPT_analyzer_store))
+  if (Args.hasArg(OPT_analyzer_store))
 Diags.Report(diag::warn_analyzer_deprecated_option) << "-analyzer-store"
 << "clang-16";
-  if (Arg *A = Args.getLastArg(OPT_analyzer_opt_analyze_nested_blocks))
+  if (Args.hasArg(OPT_analyzer_opt_analyze_nested_blocks))
 Diags.Report(diag::warn_analyzer_deprecated_option)
 << "-analyzer-opt-analyze-nested-blocks"
 << "clang-16";



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

[PATCH] D127746: [clang][dataflow] Convert `PointeeLoc` of PointerValue from reference to pointer. This allows PointeeLoc to be empty in the case of `nullptr`

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/Value.h:189
 public:
-  explicit PointerValue(StorageLocation )
+  explicit PointerValue(StorageLocation *PointeeLoc)
   : Value(Kind::Pointer), PointeeLoc(PointeeLoc) {}

Can you please document when this can be null?



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:266-267
 
-  auto  = Env.createStorageLocation(*S);
-  Env.setStorageLocation(*S, Loc);
-  Env.setValue(Loc, Env.takeOwnership(std::make_unique(
-SubExprVal->getPointeeLoc(;
+  // If PointeeLoc is null, then we are dereferencing a nullptr, skip
+  // creating a value for the dereference
+  if (auto *PointeeLoc = SubExprVal->getPointeeLoc()) {

Can you please add a test for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127746

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

[PATCH] D115674: [CMake][compiler-rt] Provide a dedicated option for LLVM unwinder

[Petr Hosek 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 rG18a1fc8459b7: [CMake][compiler-rt] Provide a dedicated 
option for LLVM unwinder (authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115674

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/scudo/standalone/CMakeLists.txt


Index: compiler-rt/lib/scudo/standalone/CMakeLists.txt
===
--- compiler-rt/lib/scudo/standalone/CMakeLists.txt
+++ compiler-rt/lib/scudo/standalone/CMakeLists.txt
@@ -36,6 +36,7 @@
 
 # We don't use the C++ standard library, so avoid including it by mistake.
 append_list_if(COMPILER_RT_HAS_NOSTDLIBXX_FLAG -nostdlib++ SCUDO_LINK_FLAGS)
+append_list_if(CXX_SUPPORTS_UNWINDLIB_NONE_FLAG --unwindlib=none 
SCUDO_LINK_FLAGS)
 
 if(ANDROID)
   list(APPEND SCUDO_CFLAGS -fno-emulated-tls)
Index: compiler-rt/cmake/config-ix.cmake
===
--- compiler-rt/cmake/config-ix.cmake
+++ compiler-rt/cmake/config-ix.cmake
@@ -4,9 +4,19 @@
 include(CheckCXXCompilerFlag)
 include(CheckIncludeFiles)
 include(CheckLibraryExists)
+include(LLVMCheckCompilerLinkerFlag)
 include(CheckSymbolExists)
 include(TestBigEndian)
 
+# The compiler driver may be implicitly trying to link against libunwind.
+# This is normally ok (libcxx relies on an unwinder), but if libunwind is
+# built in the same cmake invocation as compiler-rt and we're using the
+# in tree version of runtimes, we'd be linking against the just-built
+# libunwind (and the compiler implicit -lunwind wouldn't succeed as the newly
+# built libunwind isn't installed yet). For those cases, it'd be good to
+# link with --uwnindlib=none. Check if that option works.
+llvm_check_compiler_linker_flag(C "--unwindlib=none" 
CXX_SUPPORTS_UNWINDLIB_NONE_FLAG)
+
 check_library_exists(c fopen "" COMPILER_RT_HAS_LIBC)
 if (COMPILER_RT_USE_BUILTINS_LIBRARY)
   include(HandleCompilerRT)
Index: compiler-rt/CMakeLists.txt
===
--- compiler-rt/CMakeLists.txt
+++ compiler-rt/CMakeLists.txt
@@ -31,6 +31,7 @@
 include(SetPlatformToolchainTools)
 include(base-config-ix)
 include(CompilerRTUtils)
+include(CMakeDependentOption)
 
 option(COMPILER_RT_BUILD_BUILTINS "Build builtins" ON)
 mark_as_advanced(COMPILER_RT_BUILD_BUILTINS)
@@ -211,6 +212,11 @@
 set_property(CACHE SANITIZER_TEST_CXX PROPERTY STRINGS ;${CXXLIBS})
 handle_default_cxx_lib(SANITIZER_TEST_CXX)
 
+option(COMPILER_RT_USE_LLVM_UNWINDER "Use the LLVM unwinder." OFF)
+cmake_dependent_option(COMPILER_RT_ENABLE_STATIC_UNWINDER
+  "Statically link the LLVM unwinder." OFF
+  "COMPILER_RT_USE_LLVM_UNWINDER" OFF)
+
 set(DEFAULT_SANITIZER_USE_STATIC_LLVM_UNWINDER OFF)
 if (FUCHSIA)
   set(DEFAULT_SANITIZER_USE_STATIC_LLVM_UNWINDER ON)
@@ -504,6 +510,15 @@
 list(APPEND COMPILER_RT_COMMON_CFLAGS ${stdlib_flag})
 list(APPEND COMPILER_RT_COMMON_LINK_FLAGS ${stdlib_flag})
 
+if(COMPILER_RT_USE_LLVM_UNWINDER)
+  if (COMPILER_RT_ENABLE_STATIC_UNWINDER)
+set(unwinder_target unwind_static)
+  else()
+set(unwinder_target unwind_shared)
+  endif()
+  list(APPEND SANITIZER_CXX_ABI_LIBRARIES 
"$<$:$>")
+endif()
+
 macro(append_libcxx_libs var)
   if (${var}_INTREE)
 if (SANITIZER_USE_STATIC_LLVM_UNWINDER AND (TARGET unwind_static OR 
HAVE_LIBUNWIND))
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -112,6 +112,7 @@
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING 
"")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
@@ -179,6 +180,7 @@
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS 
${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
 set(RUNTIMES_${target}_CMAKE_SYSROOT ${FUCHSIA_${target}_SYSROOT} CACHE 
PATH "")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_HIDE_SYMBOLS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_INSTALL_STATIC_LIBRARY OFF CACHE BOOL "")


Index: 

[clang] 18a1fc8 - [CMake][compiler-rt] Provide a dedicated option for LLVM unwinder

[Petr Hosek via cfe-commits]

Author: Petr Hosek
Date: 2022-06-14T17:26:25Z
New Revision: 18a1fc8459b7fa8ca639d53e4aa5e5f5d68e8407

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

LOG: [CMake][compiler-rt] Provide a dedicated option for LLVM unwinder

This allows configuring LLVM unwinder separately from the C++ library
matching how we configure it in libcxx.

This also applies changes made to libunwind+libcxxabi+libcxx in D113253
to compiler-rt.

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

Added: 


Modified: 
clang/cmake/caches/Fuchsia-stage2.cmake
compiler-rt/CMakeLists.txt
compiler-rt/cmake/config-ix.cmake
compiler-rt/lib/scudo/standalone/CMakeLists.txt

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index 6a00556e977f4..4e3ed6086dec5 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -112,6 +112,7 @@ foreach(target 
aarch64-unknown-linux-gnu;armv7-unknown-linux-gnueabihf;i386-unkn
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING 
"")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
@@ -179,6 +180,7 @@ if(FUCHSIA_SDK)
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS 
${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
 set(RUNTIMES_${target}_CMAKE_SYSROOT ${FUCHSIA_${target}_SYSROOT} CACHE 
PATH "")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_HIDE_SYMBOLS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_INSTALL_STATIC_LIBRARY OFF CACHE BOOL "")

diff  --git a/compiler-rt/CMakeLists.txt b/compiler-rt/CMakeLists.txt
index 84575b49079c1..cdafdfc7ecc3b 100644
--- a/compiler-rt/CMakeLists.txt
+++ b/compiler-rt/CMakeLists.txt
@@ -31,6 +31,7 @@ endif()
 include(SetPlatformToolchainTools)
 include(base-config-ix)
 include(CompilerRTUtils)
+include(CMakeDependentOption)
 
 option(COMPILER_RT_BUILD_BUILTINS "Build builtins" ON)
 mark_as_advanced(COMPILER_RT_BUILD_BUILTINS)
@@ -211,6 +212,11 @@ set(CXXLIBS none default libstdc++ libc++)
 set_property(CACHE SANITIZER_TEST_CXX PROPERTY STRINGS ;${CXXLIBS})
 handle_default_cxx_lib(SANITIZER_TEST_CXX)
 
+option(COMPILER_RT_USE_LLVM_UNWINDER "Use the LLVM unwinder." OFF)
+cmake_dependent_option(COMPILER_RT_ENABLE_STATIC_UNWINDER
+  "Statically link the LLVM unwinder." OFF
+  "COMPILER_RT_USE_LLVM_UNWINDER" OFF)
+
 set(DEFAULT_SANITIZER_USE_STATIC_LLVM_UNWINDER OFF)
 if (FUCHSIA)
   set(DEFAULT_SANITIZER_USE_STATIC_LLVM_UNWINDER ON)
@@ -504,6 +510,15 @@ string(REGEX REPLACE "-stdlib=[a-zA-Z+]*" "" 
CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}
 list(APPEND COMPILER_RT_COMMON_CFLAGS ${stdlib_flag})
 list(APPEND COMPILER_RT_COMMON_LINK_FLAGS ${stdlib_flag})
 
+if(COMPILER_RT_USE_LLVM_UNWINDER)
+  if (COMPILER_RT_ENABLE_STATIC_UNWINDER)
+set(unwinder_target unwind_static)
+  else()
+set(unwinder_target unwind_shared)
+  endif()
+  list(APPEND SANITIZER_CXX_ABI_LIBRARIES 
"$<$:$>")
+endif()
+
 macro(append_libcxx_libs var)
   if (${var}_INTREE)
 if (SANITIZER_USE_STATIC_LLVM_UNWINDER AND (TARGET unwind_static OR 
HAVE_LIBUNWIND))

diff  --git a/compiler-rt/cmake/config-ix.cmake 
b/compiler-rt/cmake/config-ix.cmake
index 8cd80590258a5..cd45176cf2ba7 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -4,9 +4,19 @@ include(CheckCCompilerFlag)
 include(CheckCXXCompilerFlag)
 include(CheckIncludeFiles)
 include(CheckLibraryExists)
+include(LLVMCheckCompilerLinkerFlag)
 include(CheckSymbolExists)
 include(TestBigEndian)
 
+# The compiler driver may be implicitly trying to link against libunwind.
+# This is normally ok (libcxx relies on an unwinder), but if libunwind is
+# built in the same cmake invocation as compiler-rt and we're using the
+# in tree version of runtimes, we'd be linking against the just-built
+# libunwind (and the compiler implicit -lunwind wouldn't succeed as the newly
+# built libunwind isn't installed yet). For those cases, it'd be good to
+# link with --uwnindlib=none. Check if that option works.
+llvm_check_compiler_linker_flag(C "--unwindlib=none" 
CXX_SUPPORTS_UNWINDLIB_NONE_FLAG)
+
 check_library_exists(c fopen 

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

[Kees Cook via Phabricator via cfe-commits]

kees added a comment.

In D126864#3582360 , @nickdesaulniers 
wrote:

> If the GCC developers split this into two distinct flags, should we land 
> something we're just going to have to turn around and modify to match the new 
> flags/semantics they've created?

I'm cautious about waiting on theoretical design choices. They appear to agree 
that a "fully strict" mode should exist, and that's what -fstrict-flex-arrays 
here gets us. I'd rather get this into Clang now so I can start working through 
all the new cases it finds instead of waiting for bikeshedding to end. If Clang 
needs to rename the option in a few months, I think that's okay. But I'll leave 
it up to @serge-sans-paille


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

https://reviews.llvm.org/D126864

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

[PATCH] D127732: [analyzer][NFC] Remove unused ExprEngine::evalBinOp functions

[Balázs Benics 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 rGcf078adc90f0: [analyzer][NFC] Remove unused 
ExprEngine::evalBinOp functions (authored by steakhal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127732

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -608,17 +608,6 @@
  StmtNodeBuilder );
 
 public:
-  SVal evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op,
- NonLoc L, NonLoc R, QualType T) {
-return svalBuilder.evalBinOpNN(state, op, L, R, T);
-  }
-
-  SVal evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op,
- NonLoc L, SVal R, QualType T) {
-return R.isValid() ? svalBuilder.evalBinOpNN(state, op, L,
- R.castAs(), T) : R;
-  }
-
   SVal evalBinOp(ProgramStateRef ST, BinaryOperator::Opcode Op,
  SVal LHS, SVal RHS, QualType T) {
 return svalBuilder.evalBinOp(ST, Op, LHS, RHS, T);


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -608,17 +608,6 @@
  StmtNodeBuilder );
 
 public:
-  SVal evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op,
- NonLoc L, NonLoc R, QualType T) {
-return svalBuilder.evalBinOpNN(state, op, L, R, T);
-  }
-
-  SVal evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op,
- NonLoc L, SVal R, QualType T) {
-return R.isValid() ? svalBuilder.evalBinOpNN(state, op, L,
- R.castAs(), T) : R;
-  }
-
   SVal evalBinOp(ProgramStateRef ST, BinaryOperator::Opcode Op,
  SVal LHS, SVal RHS, QualType T) {
 return svalBuilder.evalBinOp(ST, Op, LHS, RHS, T);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] cf078ad - [analyzer][NFC] Remove unused ExprEngine::evalBinOp functions

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-06-14T19:15:20+02:00
New Revision: cf078adc90f02ad5c06162ad871035d970f072aa

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

LOG: [analyzer][NFC] Remove unused ExprEngine::evalBinOp functions

Reviewed By: martong

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 89c0a121d95f..3787f8f01b34 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -608,17 +608,6 @@ class ExprEngine {
  StmtNodeBuilder );
 
 public:
-  SVal evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op,
- NonLoc L, NonLoc R, QualType T) {
-return svalBuilder.evalBinOpNN(state, op, L, R, T);
-  }
-
-  SVal evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op,
- NonLoc L, SVal R, QualType T) {
-return R.isValid() ? svalBuilder.evalBinOpNN(state, op, L,
- R.castAs(), T) : R;
-  }
-
   SVal evalBinOp(ProgramStateRef ST, BinaryOperator::Opcode Op,
  SVal LHS, SVal RHS, QualType T) {
 return svalBuilder.evalBinOp(ST, Op, LHS, RHS, T);



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

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

If the GCC developers split this into two distinct flags, should we land 
something we're just going to have to turn around and modify to match the new 
flags/semantics they've created?


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

https://reviews.llvm.org/D126864

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

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/SemaCXX/type-traits.cpp:3099-3100
+
+  { int a[T(__is_same(remove_pointer_t, int))]; }
+  { int a[T(__is_same(remove_pointer_t, 
S))]; }
+}

cjdb wrote:
> cjdb wrote:
> > rsmith wrote:
> > > It seems strange to remove the address space qualifier here, given that 
> > > this trait doesn't remove cv-qualifiers.
> > Does `int __attribute__((address_space(1)))` make sense as a type? I 
> > thought it had to be a pointee.
> Gentle ping.
> Does int __attribute__((address_space(1))) make sense as a type? I thought it 
> had to be a pointee.

It doesn't have to be on a pointer or a pointee type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

[Kees Cook via Phabricator via cfe-commits]

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

Are the presubmit build failures unrelated?


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

https://reviews.llvm.org/D126864

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

[PATCH] D126308: cmake: use llvm dir variables for clang/utils/hmaptool

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D126308#3581240 , @dyung wrote:

> Can you take a look?

Hi @dyung , thanks for the report and sorry for the trouble!
Can you help me try to reproduce it?

Which version of Visual Studio, and what preset are you using?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126308

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

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/lib/Parse/ParseExpr.cpp:1750
+#undef TRANSFORM_TYPE_TRAIT_DEF
+if (NextToken().is(tok::less)) {
+  Tok.setKind(tok::identifier);

rsmith wrote:
> Here you're checking for `trait<` and treating it as an identifier; elsewhere 
> you're checking for `trait(` and treating it as a builtin. Is there a reason 
> to treat `trait` followed by a token other than `(` or `<` inconsistently?
The parser needs to treat `trait<` as an identifier to accommodate libstdc++'s 
usage of a few of these as alias templates. I've added a comment to explain why 
in the code.



Comment at: clang/lib/Sema/SemaType.cpp:9251
+  BaseType->isBooleanType()) {
+Diag(Loc, diag::err_make_signed_integral_only) << IsMakeSigned << BaseType;
+return QualType();

cjdb wrote:
> rsmith wrote:
> > Should we support vector types here?
> Is it a conforming extension for `make_signed_t __attribute__((ext_vector_type(2)))>` to work?
Gentle ping.



Comment at: clang/test/SemaCXX/type-traits.cpp:3099-3100
+
+  { int a[T(__is_same(remove_pointer_t, int))]; }
+  { int a[T(__is_same(remove_pointer_t, 
S))]; }
+}

cjdb wrote:
> rsmith wrote:
> > It seems strange to remove the address space qualifier here, given that 
> > this trait doesn't remove cv-qualifiers.
> Does `int __attribute__((address_space(1)))` make sense as a type? I thought 
> it had to be a pointee.
Gentle ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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