[PATCH] D132905: [clang-format] Fix a bug in inserting braces at trailing comments

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbc10ab8da1b5: [clang-format] Fix a bug in inserting braces 
at trailing comments (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132905

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25201,6 +25201,13 @@
"  f();",
Style);
 
+  verifyFormat("if (a) { //\n"
+   "  b = 1;\n"
+   "}",
+   "if (a) //\n"
+   "  b = 1;",
+   Style);
+
   verifyFormat("if (a) { // comment\n"
"  // comment\n"
"  f();\n"
@@ -25266,6 +25273,19 @@
"if (a + b > c)\n"
"  f();",
Style);
+
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
+  // TODO: Delete the following line after #57421 is fixed.
+  Style.BraceWrapping.AfterFunction = true;
+
+  verifyFormat("if (a) //\n"
+   "{\n"
+   "  b = 1;\n"
+   "}",
+   "if (a) //\n"
+   "  b = 1;",
+   Style);
 }
 
 TEST_F(FormatTest, RemoveBraces) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2606,7 +2606,10 @@
 
   if (Style.InsertBraces && !Line->InPPDirective && !Line->Tokens.empty() &&
   PreprocessorDirectives.empty()) {
-Tok = getLastNonComment(*Line);
+assert(!Line->Tokens.empty());
+Tok = Style.BraceWrapping.AfterControlStatement == FormatStyle::BWACS_Never
+  ? getLastNonComment(*Line)
+  : Line->Tokens.back().Tok;
 assert(Tok);
 if (Tok->BraceCount < 0) {
   assert(Tok->BraceCount == -1);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1869,7 +1869,7 @@
 std::string Brace;
 if (Token->BraceCount < 0) {
   assert(Token->BraceCount == -1);
-  Brace = '{';
+  Brace = "\n{";
 } else {
   Brace = '\n' + std::string(Token->BraceCount, '}');
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25201,6 +25201,13 @@
"  f();",
Style);
 
+  verifyFormat("if (a) { //\n"
+   "  b = 1;\n"
+   "}",
+   "if (a) //\n"
+   "  b = 1;",
+   Style);
+
   verifyFormat("if (a) { // comment\n"
"  // comment\n"
"  f();\n"
@@ -25266,6 +25273,19 @@
"if (a + b > c)\n"
"  f();",
Style);
+
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
+  // TODO: Delete the following line after #57421 is fixed.
+  Style.BraceWrapping.AfterFunction = true;
+
+  verifyFormat("if (a) //\n"
+   "{\n"
+   "  b = 1;\n"
+   "}",
+   "if (a) //\n"
+   "  b = 1;",
+   Style);
 }
 
 TEST_F(FormatTest, RemoveBraces) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2606,7 +2606,10 @@
 
   if (Style.InsertBraces && !Line->InPPDirective && !Line->Tokens.empty() &&
   PreprocessorDirectives.empty()) {
-Tok = getLastNonComment(*Line);
+assert(!Line->Tokens.empty());
+Tok = Style.BraceWrapping.AfterControlStatement == FormatStyle::BWACS_Never
+  ? getLastNonComment(*Line)
+  : Line->Tokens.back().Tok;
 assert(Tok);
 if (Tok->BraceCount < 0) {
   assert(Tok->BraceCount == -1);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1869,7 +1869,7 @@
 std::string Brace;
 if (Token->BraceCount < 0) {
   assert(Token->BraceCount == -1);
-  Brace = '{';
+  Brace = "\n{";
 } else {
   Brace = '\n' + std::string(Token->BraceCount, '}');
 }
___
cfe-commits mailing list

[clang] bc10ab8 - [clang-format] Fix a bug in inserting braces at trailing comments

[via cfe-commits]

Author: owenca
Date: 2022-08-30T22:50:36-07:00
New Revision: bc10ab8da1b511e794fbed2396c60c87e2e05ef2

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

LOG: [clang-format] Fix a bug in inserting braces at trailing comments

If the style wraps control statement braces, the opening braces
should be inserted after the trailing comments if present.

Fixes #57419.

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c2c020ea7ca85..1108a02407e4f 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1869,7 +1869,7 @@ class BracesInserter : public TokenAnalyzer {
 std::string Brace;
 if (Token->BraceCount < 0) {
   assert(Token->BraceCount == -1);
-  Brace = '{';
+  Brace = "\n{";
 } else {
   Brace = '\n' + std::string(Token->BraceCount, '}');
 }

diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 03c6565598a7a..1e775f70a24a1 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2606,7 +2606,10 @@ void UnwrappedLineParser::parseUnbracedBody(bool 
CheckEOF) {
 
   if (Style.InsertBraces && !Line->InPPDirective && !Line->Tokens.empty() &&
   PreprocessorDirectives.empty()) {
-Tok = getLastNonComment(*Line);
+assert(!Line->Tokens.empty());
+Tok = Style.BraceWrapping.AfterControlStatement == FormatStyle::BWACS_Never
+  ? getLastNonComment(*Line)
+  : Line->Tokens.back().Tok;
 assert(Tok);
 if (Tok->BraceCount < 0) {
   assert(Tok->BraceCount == -1);

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 257be01c3dae2..5d1129b851dff 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -25201,6 +25201,13 @@ TEST_F(FormatTest, InsertBraces) {
"  f();",
Style);
 
+  verifyFormat("if (a) { //\n"
+   "  b = 1;\n"
+   "}",
+   "if (a) //\n"
+   "  b = 1;",
+   Style);
+
   verifyFormat("if (a) { // comment\n"
"  // comment\n"
"  f();\n"
@@ -25266,6 +25273,19 @@ TEST_F(FormatTest, InsertBraces) {
"if (a + b > c)\n"
"  f();",
Style);
+
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
+  // TODO: Delete the following line after #57421 is fixed.
+  Style.BraceWrapping.AfterFunction = true;
+
+  verifyFormat("if (a) //\n"
+   "{\n"
+   "  b = 1;\n"
+   "}",
+   "if (a) //\n"
+   "  b = 1;",
+   Style);
 }
 
 TEST_F(FormatTest, RemoveBraces) {



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

[PATCH] D132911: [clang-format] Fix annotating when deleting array of pointers

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added a comment.

Please mark https://reviews.llvm.org/D132911#inline-1279832 as done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132911

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

[clang] fed71b0 - [NFC] Add an invalid test case for clang/test/CXX/module/module.reach/ex1.cpp

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-08-31T13:02:00+08:00
New Revision: fed71b04fb3cc6b1a5a21f64c26104962d816300

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

LOG: [NFC] Add an invalid test case for 
clang/test/CXX/module/module.reach/ex1.cpp

Added: 


Modified: 
clang/test/CXX/module/module.reach/ex1.cpp

Removed: 




diff  --git a/clang/test/CXX/module/module.reach/ex1.cpp 
b/clang/test/CXX/module/module.reach/ex1.cpp
index 00f87607dc1d8..a1e38c4c88a28 100644
--- a/clang/test/CXX/module/module.reach/ex1.cpp
+++ b/clang/test/CXX/module/module.reach/ex1.cpp
@@ -41,3 +41,6 @@ B b3; // expected-error {{definition of 'B' must be imported 
from module 'M:B' b
   // expected-note@* {{definition here is not reachable}} expected-note@* 
{{}}
 // FIXME: We should emit an error for unreachable definition of B.
 void g() { f(); }
+void g1() { f(B()); } // expected-error 1+{{definition of 'B' must be imported 
from module 'M:B' before it is required}}
+  // expected-note@* 1+{{definition here is not reachable}}
+  // expected-n...@m.cppm:5 {{passing argument to 
parameter 'b' here}}



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

[PATCH] D132991: [Clang] Give error message for invalid profile path when compiling IR

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added reviewers: xur, vsk.
aidengrossman added a subscriber: mtrofin.
aidengrossman added a comment.

Pinging reviewers based on previous commits in `setPGOUseInstrumentor`.

If there is an alternative method of writing this patch (eg placing the check 
in an equivalent of `CodeGenModule` for the IR side), let me know and I can 
adjust it. I'm not very familiar with the clang codebase, so there might be 
some inefficiencies in how I've implemented things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132991

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

[PATCH] D132991: [Clang] Give error message for invalid profile path when compiling IR

[Aiden Grossman via Phabricator via cfe-commits]

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

Before this patch, when compiling an IR file (eg the .llvmbc section
from an object file compiled with -Xclang -fembed-bitcode=all) and
profile data was passed in using the -fprofile-instrument-use-path
flag, there would be no error printed (as the previous implementation
relied on the error getting caught again in the constructor of
CodeGenModule which isn't called when -x ir is set). This patch
moves the error checking directly to where the error is caught
originally rather than failing silently in setPGOUseInstrumentor and
waiting to catch it in CodeGenModule to print diagnostic information to
the user.

Regression test added.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132991

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/profile-does-not-exist-ir.c


Index: clang/test/Profile/profile-does-not-exist-ir.c
===
--- /dev/null
+++ clang/test/Profile/profile-does-not-exist-ir.c
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -emit-llvm -x ir %s -o - 
-fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
+
+// CHECK: error: Could not read profile {{.*}}.nonexistent.profdata:
+// CHECK-NOT: Assertion failed
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1293,12 +1293,15 @@
 
 // Set the profile kind using fprofile-instrument-use-path.
 static void setPGOUseInstrumentor(CodeGenOptions ,
-  const Twine ) {
+  const Twine ,
+  DiagnosticsEngine ) {
   auto ReaderOrErr = llvm::IndexedInstrProfReader::create(ProfileName);
-  // In error, return silently and let Clang PGOUse report the error message.
   if (auto E = ReaderOrErr.takeError()) {
-llvm::consumeError(std::move(E));
-Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
+unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
+"Could not read profile %0: %1");
+llvm::handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase ) {
+  Diags.Report(DiagID) << ProfileName.str() << EI.message();
+});
 return;
   }
   std::unique_ptr PGOReader =
@@ -1717,7 +1720,7 @@
   }
 
   if (!Opts.ProfileInstrumentUsePath.empty())
-setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath);
+setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath, Diags);
 
   if (const Arg *A = Args.getLastArg(OPT_ftime_report, OPT_ftime_report_EQ)) {
 Opts.TimePasses = true;


Index: clang/test/Profile/profile-does-not-exist-ir.c
===
--- /dev/null
+++ clang/test/Profile/profile-does-not-exist-ir.c
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -emit-llvm -x ir %s -o - -fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
+
+// CHECK: error: Could not read profile {{.*}}.nonexistent.profdata:
+// CHECK-NOT: Assertion failed
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1293,12 +1293,15 @@
 
 // Set the profile kind using fprofile-instrument-use-path.
 static void setPGOUseInstrumentor(CodeGenOptions ,
-  const Twine ) {
+  const Twine ,
+  DiagnosticsEngine ) {
   auto ReaderOrErr = llvm::IndexedInstrProfReader::create(ProfileName);
-  // In error, return silently and let Clang PGOUse report the error message.
   if (auto E = ReaderOrErr.takeError()) {
-llvm::consumeError(std::move(E));
-Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
+unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
+"Could not read profile %0: %1");
+llvm::handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase ) {
+  Diags.Report(DiagID) << ProfileName.str() << EI.message();
+});
 return;
   }
   std::unique_ptr PGOReader =
@@ -1717,7 +1720,7 @@
   }
 
   if (!Opts.ProfileInstrumentUsePath.empty())
-setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath);
+setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath, Diags);
 
   if (const Arg *A = Args.getLastArg(OPT_ftime_report, OPT_ftime_report_EQ)) {
 Opts.TimePasses = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D132990: [Clang] Fix compat diagnostic to detect a nontype template parameter has a placeholder type using getContainedAutoType()

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik created this revision.
shafik added reviewers: aaron.ballman, erichkeane, rsmith.
Herald added a project: All.
shafik requested review of this revision.

Based on the changes introduced by 15361a21e01026e74cb17011b702c7d1c881ae94 it 
looks like C++17 compatibility diagnostic should have been checking 
`getContainedAutoType()`.

This fixes: https://github.com/llvm/llvm-project/issues/57369


https://reviews.llvm.org/D132990

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wpre-c++17-compat %s
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+
+template  // expected-warning {{non-type template parameters declared 
with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
+
+template  // expected-warning {{non-type template parameters 
declared with 'decltype(auto)' are incompatible with C++ standards before 
C++17}}
+struct B{};
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,7 +1531,7 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
+  if (TInfo->getType()->getContainedAutoType()) {
 Diag(D.getIdentifierLoc(),
  diag::warn_cxx14_compat_template_nontype_parm_auto_type)
   << QualType(TInfo->getType()->getContainedAutoType(), 0);


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wpre-c++17-compat %s
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+
+template  // expected-warning {{non-type template parameters declared with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
+
+template  // expected-warning {{non-type template parameters declared with 'decltype(auto)' are incompatible with C++ standards before C++17}}
+struct B{};
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,7 +1531,7 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
+  if (TInfo->getType()->getContainedAutoType()) {
 Diag(D.getIdentifierLoc(),
  diag::warn_cxx14_compat_template_nontype_parm_auto_type)
   << QualType(TInfo->getType()->getContainedAutoType(), 0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129488: [Sema] Delay evaluation of std::source_location::current() in default arguments

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

There is another example we shouldn't make this specific for 
std::source_location::current(): 
https://github.com/llvm/llvm-project/issues/57459. I guess we can solve the 
issue too if we evaluate default argument at the caller position.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129488

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

[clang] b1d5af8 - [docs] Add "Standard C++ Modules"

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-08-31T11:09:46+08:00
New Revision: b1d5af81249dc7e5697faf9ee33f86012ccd8668

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

LOG: [docs] Add "Standard C++ Modules"

We get some standard C++ module things done in clang15.x. But we lack a
user documentation for it. The implementation of standard C++ modules
share a big part of codes with clang modules. But they have very
different semantics and user interfaces, so I think it is necessary to
add a document for Standard C++ modules. Previously, there were also
some people ask the document for standard C++ Modules and I couldn't
offer that time.

Reviewed By: iains, Mordante, h-vetinari, ruoso, dblaikie, JohelEGP,
aaronmondal

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

Added: 
clang/docs/StandardCPlusPlusModules.rst

Modified: 
clang/docs/index.rst

Removed: 




diff  --git a/clang/docs/StandardCPlusPlusModules.rst 
b/clang/docs/StandardCPlusPlusModules.rst
new file mode 100644
index ..ae434b14ef50
--- /dev/null
+++ b/clang/docs/StandardCPlusPlusModules.rst
@@ -0,0 +1,876 @@
+
+Standard C++ Modules
+
+
+.. contents::
+   :local:
+
+Introduction
+
+
+The term ``modules`` has a lot of meanings. For the users of Clang, modules may
+refer to ``Objective-C Modules``, ``Clang C++ Modules`` (or ``Clang Header 
Modules``,
+etc.) or ``Standard C++ Modules``. The implementation of all these kinds of 
modules in Clang
+has a lot of shared code, but from the perspective of users, their semantics 
and
+command line interfaces are very 
diff erent. This document focuses on
+an introduction of how to use standard C++ modules in Clang.
+
+There is already a detailed document about `Clang modules `_, it
+should be helpful to read `Clang modules `_ if you want to know
+more about the general idea of modules. Since standard C++ modules have 
diff erent semantics
+(and work flows) from `Clang modules`, this page describes the background and 
use of
+Clang with standard C++ modules.
+
+Modules exist in two forms in the C++ Language Specification. They can refer to
+either "Named Modules" or to "Header Units". This document covers both forms.
+
+Standard C++ Named modules
+==
+
+This document was intended to be a manual first and foremost, however, we 
consider it helpful to
+introduce some language background here for readers who are not familiar with
+the new language feature. This document is not intended to be a language
+tutorial; it will only introduce necessary concepts about the
+structure and building of the project.
+
+Background and terminology
+--
+
+Modules
+~~~
+
+In this document, the term ``Modules``/``modules`` refers to standard C++ 
modules
+feature if it is not decorated by ``Clang``.
+
+Clang Modules
+~
+
+In this document, the term ``Clang Modules``/``Clang modules`` refer to Clang
+c++ modules extension. These are also known as ``Clang header modules``,
+``Clang module map modules`` or ``Clang c++ modules``.
+
+Module and module unit
+~~
+
+A module consists of one or more module units. A module unit is a special
+translation unit. Every module unit must have a module declaration. The syntax
+of the module declaration is:
+
+.. code-block:: c++
+
+  [export] module module_name[:partition_name];
+
+Terms enclosed in ``[]`` are optional. The syntax of ``module_name`` and 
``partition_name``
+in regex form corresponds to ``[a-zA-Z_][a-zA-Z_0-9\.]*``. In particular, a 
literal dot ``.``
+in the name has no semantic meaning (e.g. implying a hierarchy).
+
+In this document, module units are classified into:
+
+* Primary module interface unit.
+
+* Module implementation unit.
+
+* Module interface partition unit.
+
+* Internal module partition unit.
+
+A primary module interface unit is a module unit whose module declaration is
+``export module module_name;``. The ``module_name`` here denotes the name of 
the
+module. A module should have one and only one primary module interface unit.
+
+A module implementation unit is a module unit whose module declaration is
+``module module_name;``. A module could have multiple module implementation
+units with the same declaration.
+
+A module interface partition unit is a module unit whose module declaration is
+``export module module_name:partition_name;``. The ``partition_name`` should be
+unique within any given module.
+
+An internal module partition unit is a module unit whose module declaration
+is ``module module_name:partition_name;``. The ``partition_name`` should be
+unique within any given module.
+
+In this document, we use the following umbrella terms:
+
+* A ``module interface unit`` refers to 

[PATCH] D131388: [docs] Add "Standard C++ Modules"

[Chuanqi Xu via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb1d5af81249d: [docs] Add Standard C++ Modules 
(authored by ChuanqiXu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131388

Files:
  clang/docs/StandardCPlusPlusModules.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -40,6 +40,7 @@
SafeStack
ShadowCallStack
SourceBasedCodeCoverage
+   StandardCPlusPlusModules
Modules
MSVCCompatibility
MisExpect
Index: clang/docs/StandardCPlusPlusModules.rst
===
--- /dev/null
+++ clang/docs/StandardCPlusPlusModules.rst
@@ -0,0 +1,876 @@
+
+Standard C++ Modules
+
+
+.. contents::
+   :local:
+
+Introduction
+
+
+The term ``modules`` has a lot of meanings. For the users of Clang, modules may
+refer to ``Objective-C Modules``, ``Clang C++ Modules`` (or ``Clang Header Modules``,
+etc.) or ``Standard C++ Modules``. The implementation of all these kinds of modules in Clang
+has a lot of shared code, but from the perspective of users, their semantics and
+command line interfaces are very different. This document focuses on
+an introduction of how to use standard C++ modules in Clang.
+
+There is already a detailed document about `Clang modules `_, it
+should be helpful to read `Clang modules `_ if you want to know
+more about the general idea of modules. Since standard C++ modules have different semantics
+(and work flows) from `Clang modules`, this page describes the background and use of
+Clang with standard C++ modules.
+
+Modules exist in two forms in the C++ Language Specification. They can refer to
+either "Named Modules" or to "Header Units". This document covers both forms.
+
+Standard C++ Named modules
+==
+
+This document was intended to be a manual first and foremost, however, we consider it helpful to
+introduce some language background here for readers who are not familiar with
+the new language feature. This document is not intended to be a language
+tutorial; it will only introduce necessary concepts about the
+structure and building of the project.
+
+Background and terminology
+--
+
+Modules
+~~~
+
+In this document, the term ``Modules``/``modules`` refers to standard C++ modules
+feature if it is not decorated by ``Clang``.
+
+Clang Modules
+~
+
+In this document, the term ``Clang Modules``/``Clang modules`` refer to Clang
+c++ modules extension. These are also known as ``Clang header modules``,
+``Clang module map modules`` or ``Clang c++ modules``.
+
+Module and module unit
+~~
+
+A module consists of one or more module units. A module unit is a special
+translation unit. Every module unit must have a module declaration. The syntax
+of the module declaration is:
+
+.. code-block:: c++
+
+  [export] module module_name[:partition_name];
+
+Terms enclosed in ``[]`` are optional. The syntax of ``module_name`` and ``partition_name``
+in regex form corresponds to ``[a-zA-Z_][a-zA-Z_0-9\.]*``. In particular, a literal dot ``.``
+in the name has no semantic meaning (e.g. implying a hierarchy).
+
+In this document, module units are classified into:
+
+* Primary module interface unit.
+
+* Module implementation unit.
+
+* Module interface partition unit.
+
+* Internal module partition unit.
+
+A primary module interface unit is a module unit whose module declaration is
+``export module module_name;``. The ``module_name`` here denotes the name of the
+module. A module should have one and only one primary module interface unit.
+
+A module implementation unit is a module unit whose module declaration is
+``module module_name;``. A module could have multiple module implementation
+units with the same declaration.
+
+A module interface partition unit is a module unit whose module declaration is
+``export module module_name:partition_name;``. The ``partition_name`` should be
+unique within any given module.
+
+An internal module partition unit is a module unit whose module declaration
+is ``module module_name:partition_name;``. The ``partition_name`` should be
+unique within any given module.
+
+In this document, we use the following umbrella terms:
+
+* A ``module interface unit`` refers to either a ``primary module interface unit``
+  or a ``module interface partition unit``.
+
+* An ``importable module unit`` refers to either a ``module interface unit``
+  or a ``internal module partition unit``.
+
+* A ``module partition unit`` refers to either a ``module interface partition unit``
+  or a ``internal module partition unit``.
+
+Built Module Interface file

[PATCH] D131388: [docs] Add "Standard C++ Modules"

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added a comment.

I'm going to land this. Thanks for everyone who reviewed this!


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

https://reviews.llvm.org/D131388

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

[PATCH] D132907: [msan] Add more specific messages for use-after-destroy

[Vitaly Buka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc059ede28ea8: [msan] Add more specific messages for 
use-after-destroy (authored by vitalybuka).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132907

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/test/CodeGenCXX/sanitize-dtor-bit-field.cpp
  clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
  clang/test/CodeGenCXX/sanitize-dtor-derived-class.cpp
  clang/test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
  clang/test/CodeGenCXX/sanitize-dtor-tail-call.cpp
  clang/test/CodeGenCXX/sanitize-dtor-trivial-base.cpp
  clang/test/CodeGenCXX/sanitize-dtor-trivial.cpp
  clang/test/CodeGenCXX/sanitize-dtor-vtable.cpp
  clang/test/CodeGenCXX/sanitize-dtor-zero-size-field.cpp
  compiler-rt/include/sanitizer/msan_interface.h
  compiler-rt/lib/msan/msan.h
  compiler-rt/lib/msan/msan_interceptors.cpp
  compiler-rt/lib/msan/msan_interface_internal.h
  compiler-rt/lib/msan/msan_report.cpp
  compiler-rt/test/msan/dtor-base-access.cpp
  compiler-rt/test/msan/dtor-vtable-multiple-inheritance.cpp
  compiler-rt/test/msan/use-after-dtor.cpp

Index: compiler-rt/test/msan/use-after-dtor.cpp
===
--- compiler-rt/test/msan/use-after-dtor.cpp
+++ compiler-rt/test/msan/use-after-dtor.cpp
@@ -32,7 +32,7 @@
   // CHECK-UAD: WARNING: MemorySanitizer: use-of-uninitialized-value
   // CHECK-UAD: {{#0 0x.* in main.*use-after-dtor.cpp:}}[[@LINE-3]]
 
-  // CHECK-ORIGINS: Memory was marked as uninitialized
+  // CHECK-ORIGINS: Member fields were destroyed
   // CHECK-ORIGINS: {{#0 0x.* in __sanitizer_dtor_callback}}
   // CHECK-ORIGINS: {{#1 0x.* in .*~Simple.*cpp:}}[[@LINE-18]]:
 
Index: compiler-rt/test/msan/dtor-vtable-multiple-inheritance.cpp
===
--- compiler-rt/test/msan/dtor-vtable-multiple-inheritance.cpp
+++ compiler-rt/test/msan/dtor-vtable-multiple-inheritance.cpp
@@ -51,8 +51,8 @@
   // This fails
 #ifdef CVPTR
   c->A_Foo();
-// CVPTR: Memory was marked as uninitialized
-// CVPTR: {{#0 0x.* in __sanitizer_dtor_callback}}
+// CVPTR: Virtual table ptr was destroyed
+// CVPTR: {{#0 0x.* in __sanitizer_dtor_callback_vptr}}
 // CVPTR: {{#1 0x.* in ~C .*cpp:}}[[@LINE-28]]:
 // CVPTR: {{#2 0x.* in main .*cpp:}}[[@LINE-7]]:
 #endif
@@ -63,16 +63,16 @@
   // Both of these fail
 #ifdef EAVPTR
   e->A_Foo();
-// EAVPTR: Memory was marked as uninitialized
-// EAVPTR: {{#0 0x.* in __sanitizer_dtor_callback}}
+// EAVPTR: Virtual table ptr was destroyed
+// EAVPTR: {{#0 0x.* in __sanitizer_dtor_callback_vptr}}
 // EAVPTR: {{#1 0x.* in ~E .*cpp:}}[[@LINE-25]]:
 // EAVPTR: {{#2 0x.* in main .*cpp:}}[[@LINE-7]]:
 #endif
 
 #ifdef EDVPTR
   e->D_Foo();
-// EDVPTR: Memory was marked as uninitialized
-// EDVPTR: {{#0 0x.* in __sanitizer_dtor_callback}}
+// EDVPTR: Virtual table ptr was destroyed
+// EDVPTR: {{#0 0x.* in __sanitizer_dtor_callback_vptr}}
 // EDVPTR: {{#1 0x.* in ~E .*cpp:}}[[@LINE-33]]:
 // EDVPTR: {{#2 0x.* in main .*cpp:}}[[@LINE-15]]:
 #endif
Index: compiler-rt/test/msan/dtor-base-access.cpp
===
--- compiler-rt/test/msan/dtor-base-access.cpp
+++ compiler-rt/test/msan/dtor-base-access.cpp
@@ -66,17 +66,17 @@
   assert(__msan_test_shadow(>d, sizeof(g->d)) == 0);
 
   __msan_print_shadow(>tb0, sizeof(g->tb0));
-  // CHECK: Memory was marked as uninitialized
+  // CHECK: Member fields were destroyed
   // CHECK: {{#0 0x.* in __sanitizer_dtor_callback}}
   // CHECK: {{#1 0x.* in .*~Derived.*cpp:}}[[@LINE-20]]:
 
   __msan_print_shadow(>b, sizeof(g->b));
-  // CHECK: Memory was marked as uninitialized
+  // CHECK: Member fields were destroyed
   // CHECK: {{#0 0x.* in __sanitizer_dtor_callback}}
   // CHECK: {{#1 0x.* in .*~Base.*cpp:}}[[@LINE-33]]:
 
   __msan_print_shadow(>tb1, sizeof(g->tb1));
-  // CHECK: Memory was marked as uninitialized
+  // CHECK: Member fields were destroyed
   // CHECK: {{#0 0x.* in __sanitizer_dtor_callback}}
   // CHECK: {{#1 0x.* in .*~Derived.*cpp:}}[[@LINE-30]]:
 
Index: compiler-rt/lib/msan/msan_report.cpp
===
--- compiler-rt/lib/msan/msan_report.cpp
+++ compiler-rt/lib/msan/msan_report.cpp
@@ -81,6 +81,13 @@
 Printf("  %sMemory was marked as uninitialized%s\n", d.Origin(),
d.Default());
 break;
+  case STACK_TRACE_TAG_FIELDS:
+Printf("  %sMember fields were destroyed%s\n", d.Origin(), d.Default());
+break;
+  case STACK_TRACE_TAG_VPTR:
+Printf("  %sVirtual table ptr was destroyed%s\n", d.Origin(),
+   d.Default());
+break;
   default:
 Printf("  %sUninitialized value was created%s\n", d.Origin(),
d.Default());
Index: 

[clang] c059ede - [msan] Add more specific messages for use-after-destroy

[Vitaly Buka via cfe-commits]

Author: Vitaly Buka
Date: 2022-08-30T19:52:32-07:00
New Revision: c059ede28ea8faf0540cedad74bc5698ec59e744

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

LOG: [msan] Add more specific messages for use-after-destroy

Reviewed By: kda, kstoimenov

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

Added: 


Modified: 
clang/lib/CodeGen/CGClass.cpp
clang/test/CodeGenCXX/sanitize-dtor-bit-field.cpp
clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
clang/test/CodeGenCXX/sanitize-dtor-derived-class.cpp
clang/test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
clang/test/CodeGenCXX/sanitize-dtor-tail-call.cpp
clang/test/CodeGenCXX/sanitize-dtor-trivial-base.cpp
clang/test/CodeGenCXX/sanitize-dtor-trivial.cpp
clang/test/CodeGenCXX/sanitize-dtor-vtable.cpp
clang/test/CodeGenCXX/sanitize-dtor-zero-size-field.cpp
compiler-rt/include/sanitizer/msan_interface.h
compiler-rt/lib/msan/msan.h
compiler-rt/lib/msan/msan_interceptors.cpp
compiler-rt/lib/msan/msan_interface_internal.h
compiler-rt/lib/msan/msan_report.cpp
compiler-rt/test/msan/dtor-base-access.cpp
compiler-rt/test/msan/dtor-vtable-multiple-inheritance.cpp
compiler-rt/test/msan/use-after-dtor.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 703eff8ac54c5..ec4d8e50a3560 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -1649,23 +1649,35 @@ namespace {
 }
   };
 
-  static void EmitSanitizerDtorCallback(CodeGenFunction , llvm::Value *Ptr,
-CharUnits::QuantityType PoisonSize) {
+  static void EmitSanitizerDtorCallback(
+  CodeGenFunction , StringRef Name, llvm::Value *Ptr,
+  llvm::Optional PoisonSize = {}) {
 CodeGenFunction::SanitizerScope SanScope();
 // Pass in void pointer and size of region as arguments to runtime
 // function
-llvm::Value *Args[] = {CGF.Builder.CreateBitCast(Ptr, CGF.VoidPtrTy),
-   llvm::ConstantInt::get(CGF.SizeTy, PoisonSize)};
+SmallVector Args = {
+CGF.Builder.CreateBitCast(Ptr, CGF.VoidPtrTy)};
+SmallVector ArgTypes = {CGF.VoidPtrTy};
 
-llvm::Type *ArgTypes[] = {CGF.VoidPtrTy, CGF.SizeTy};
+if (PoisonSize.has_value()) {
+  Args.emplace_back(llvm::ConstantInt::get(CGF.SizeTy, *PoisonSize));
+  ArgTypes.emplace_back(CGF.SizeTy);
+}
 
 llvm::FunctionType *FnType =
 llvm::FunctionType::get(CGF.VoidTy, ArgTypes, false);
-llvm::FunctionCallee Fn =
-CGF.CGM.CreateRuntimeFunction(FnType, "__sanitizer_dtor_callback");
+llvm::FunctionCallee Fn = CGF.CGM.CreateRuntimeFunction(FnType, Name);
+
 CGF.EmitNounwindRuntimeCall(Fn, Args);
   }
 
+  static void
+  EmitSanitizerDtorFieldsCallback(CodeGenFunction , llvm::Value *Ptr,
+  CharUnits::QuantityType PoisonSize) {
+EmitSanitizerDtorCallback(CGF, "__sanitizer_dtor_callback_fields", Ptr,
+  PoisonSize);
+  }
+
   /// Poison base class with a trivial destructor.
   struct SanitizeDtorTrivialBase final : EHScopeStack::Cleanup {
 const CXXRecordDecl *BaseClass;
@@ -1687,7 +1699,8 @@ namespace {
   if (!BaseSize.isPositive())
 return;
 
-  EmitSanitizerDtorCallback(CGF, Addr.getPointer(), 
BaseSize.getQuantity());
+  EmitSanitizerDtorFieldsCallback(CGF, Addr.getPointer(),
+  BaseSize.getQuantity());
 
   // Prevent the current stack frame from disappearing from the stack 
trace.
   CGF.CurFn->addFnAttr("disable-tail-calls", "true");
@@ -1735,7 +1748,7 @@ namespace {
   if (!PoisonSize.isPositive())
 return;
 
-  EmitSanitizerDtorCallback(CGF, OffsetPtr, PoisonSize.getQuantity());
+  EmitSanitizerDtorFieldsCallback(CGF, OffsetPtr, 
PoisonSize.getQuantity());
 
   // Prevent the current stack frame from disappearing from the stack 
trace.
   CGF.CurFn->addFnAttr("disable-tail-calls", "true");
@@ -1752,15 +1765,13 @@ namespace {
 void Emit(CodeGenFunction , Flags flags) override {
   assert(Dtor->getParent()->isDynamicClass());
   (void)Dtor;
-  ASTContext  = CGF.getContext();
   // Poison vtable and vtable ptr if they exist for this class.
   llvm::Value *VTablePtr = CGF.LoadCXXThis();
 
-  CharUnits::QuantityType PoisonSize =
-  Context.toCharUnitsFromBits(CGF.PointerWidthInBits).getQuantity();
   // Pass in void pointer and size of region as arguments to runtime
   // function
-  EmitSanitizerDtorCallback(CGF, VTablePtr, PoisonSize);
+  EmitSanitizerDtorCallback(CGF, "__sanitizer_dtor_callback_vptr",
+VTablePtr);
 

[PATCH] D132907: [msan] Add more specific messages for use-after-destroy

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added inline comments.



Comment at: clang/lib/CodeGen/CGClass.cpp:1773
   // function
-  EmitSanitizerDtorCallback(CGF, VTablePtr, PoisonSize);
+  EmitSanitizerDtorCallback(CGF, "__sanitizer_dtor_callback_vptr",
+VTablePtr);

kstoimenov wrote:
> Should this be a constant? 
it would be nice if we can share constants with runtime, but as is 
clang/compiler-rt do not include each other
without reuse I don't see a point in named constant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132907

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

[PATCH] D132819: [RISCV] Add MC support of RISCV zcmp Extension

[Liao Chunyu via Phabricator via cfe-commits]

liaolucy added a comment.

I found a lot of ZCE/zce, do we need to change ZCE to ZCMP?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132819

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

[PATCH] D131388: [docs] Add "Standard C++ Modules"

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

@bbrown105 I am going to land this today. it looks like you're not objecting it 
and we don't have the time to wait for your formal approval...


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

https://reviews.llvm.org/D131388

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

[PATCH] D131388: [docs] Add "Standard C++ Modules"

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D131388#3758898 , @aaronmondal 
wrote:

> I just noticed that there probably needs to be one change. The doc sometimes 
> describes `-fprebuilt-module-interface`, which is not a valid option. I think 
> the occurrences were meant to be `-fpbrebuilt-module-path`.

Oh, thanks for noticing. It is weird that I made such a mistake...


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

https://reviews.llvm.org/D131388

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

[PATCH] D131388: [docs] Add "Standard C++ Modules"

[Chuanqi Xu via Phabricator via cfe-commits]

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

Address comments.


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

https://reviews.llvm.org/D131388

Files:
  clang/docs/StandardCPlusPlusModules.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -40,6 +40,7 @@
SafeStack
ShadowCallStack
SourceBasedCodeCoverage
+   StandardCPlusPlusModules
Modules
MSVCCompatibility
MisExpect
Index: clang/docs/StandardCPlusPlusModules.rst
===
--- /dev/null
+++ clang/docs/StandardCPlusPlusModules.rst
@@ -0,0 +1,876 @@
+
+Standard C++ Modules
+
+
+.. contents::
+   :local:
+
+Introduction
+
+
+The term ``modules`` has a lot of meanings. For the users of Clang, modules may
+refer to ``Objective-C Modules``, ``Clang C++ Modules`` (or ``Clang Header Modules``,
+etc.) or ``Standard C++ Modules``. The implementation of all these kinds of modules in Clang
+has a lot of shared code, but from the perspective of users, their semantics and
+command line interfaces are very different. This document focuses on
+an introduction of how to use standard C++ modules in Clang.
+
+There is already a detailed document about `Clang modules `_, it
+should be helpful to read `Clang modules `_ if you want to know
+more about the general idea of modules. Since standard C++ modules have different semantics
+(and work flows) from `Clang modules`, this page describes the background and use of
+Clang with standard C++ modules.
+
+Modules exist in two forms in the C++ Language Specification. They can refer to
+either "Named Modules" or to "Header Units". This document covers both forms.
+
+Standard C++ Named modules
+==
+
+This document was intended to be a manual first and foremost, however, we consider it helpful to
+introduce some language background here for readers who are not familiar with
+the new language feature. This document is not intended to be a language
+tutorial; it will only introduce necessary concepts about the
+structure and building of the project.
+
+Background and terminology
+--
+
+Modules
+~~~
+
+In this document, the term ``Modules``/``modules`` refers to standard C++ modules
+feature if it is not decorated by ``Clang``.
+
+Clang Modules
+~
+
+In this document, the term ``Clang Modules``/``Clang modules`` refer to Clang
+c++ modules extension. These are also known as ``Clang header modules``,
+``Clang module map modules`` or ``Clang c++ modules``.
+
+Module and module unit
+~~
+
+A module consists of one or more module units. A module unit is a special
+translation unit. Every module unit must have a module declaration. The syntax
+of the module declaration is:
+
+.. code-block:: c++
+
+  [export] module module_name[:partition_name];
+
+Terms enclosed in ``[]`` are optional. The syntax of ``module_name`` and ``partition_name``
+in regex form corresponds to ``[a-zA-Z_][a-zA-Z_0-9\.]*``. In particular, a literal dot ``.``
+in the name has no semantic meaning (e.g. implying a hierarchy).
+
+In this document, module units are classified into:
+
+* Primary module interface unit.
+
+* Module implementation unit.
+
+* Module interface partition unit.
+
+* Internal module partition unit.
+
+A primary module interface unit is a module unit whose module declaration is
+``export module module_name;``. The ``module_name`` here denotes the name of the
+module. A module should have one and only one primary module interface unit.
+
+A module implementation unit is a module unit whose module declaration is
+``module module_name;``. A module could have multiple module implementation
+units with the same declaration.
+
+A module interface partition unit is a module unit whose module declaration is
+``export module module_name:partition_name;``. The ``partition_name`` should be
+unique within any given module.
+
+An internal module partition unit is a module unit whose module declaration
+is ``module module_name:partition_name;``. The ``partition_name`` should be
+unique within any given module.
+
+In this document, we use the following umbrella terms:
+
+* A ``module interface unit`` refers to either a ``primary module interface unit``
+  or a ``module interface partition unit``.
+
+* An ``importable module unit`` refers to either a ``module interface unit``
+  or a ``internal module partition unit``.
+
+* A ``module partition unit`` refers to either a ``module interface partition unit``
+  or a ``internal module partition unit``.
+
+Built Module Interface file
+~~~
+
+A ``Built Module Interface file`` stands for the precompiled result of an importable module unit.
+It is also called the acronym ``BMI`` genrally.
+
+Global module 

[PATCH] D132911: [clang-format] Fix annotating when deleting array of pointers

[Jack Huang via Phabricator via cfe-commits]

jackhong12 marked 7 inline comments as done.
jackhong12 added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2375-2378
+const FormatToken *Prev = PrevToken;
+if (Prev->is(tok::r_square) && (Prev = Prev->getPreviousNonComment()) &&
+Prev->is(tok::l_square) && (Prev = Prev->getPreviousNonComment()) &&
+Prev->is(tok::kw_delete))

owenpan wrote:
> `endsSequence()` is exactly what you want here. ;)
Thanks! It looks more concise now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132911

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

[PATCH] D132911: [clang-format] Fix annotating when deleting array of pointers

[Jack Huang via Phabricator via cfe-commits]

jackhong12 updated this revision to Diff 456836.
jackhong12 retitled this revision from "Fix annotating when deleting array of 
pointers" to "[clang-format] Fix annotating when deleting array of pointers".
jackhong12 edited the summary of this revision.
jackhong12 added a comment.

Add left alignment test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132911

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -120,6 +120,17 @@
   Tokens = annotate("int i = int{42} * 2;");
   EXPECT_EQ(Tokens.size(), 11u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("delete[] *ptr;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
+  Tokens = annotate("delete[] **ptr;");
+  EXPECT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::star, TT_UnaryOperator);
+  Tokens = annotate("delete[] *(ptr);");
+  EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10535,6 +10535,11 @@
"} & = {};",
Style);
 
+  Style.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("delete[] *ptr;", Style);
+  verifyFormat("delete[] **ptr;", Style);
+  verifyFormat("delete[] *(ptr);", Style);
+
   verifyIndependentOfContext("MACRO(int *i);");
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2372,6 +2372,9 @@
 !PrevToken->MatchingParen)
   return TT_PointerOrReference;
 
+if (PrevToken->endsSequence(tok::r_square, tok::l_square, tok::kw_delete))
+  return TT_UnaryOperator;
+
 if (PrevToken->Tok.isLiteral() ||
 PrevToken->isOneOf(tok::r_paren, tok::r_square, tok::kw_true,
tok::kw_false, tok::r_brace)) {


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -120,6 +120,17 @@
   Tokens = annotate("int i = int{42} * 2;");
   EXPECT_EQ(Tokens.size(), 11u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("delete[] *ptr;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
+  Tokens = annotate("delete[] **ptr;");
+  EXPECT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::star, TT_UnaryOperator);
+  Tokens = annotate("delete[] *(ptr);");
+  EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10535,6 +10535,11 @@
"} & = {};",
Style);
 
+  Style.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("delete[] *ptr;", Style);
+  verifyFormat("delete[] **ptr;", Style);
+  verifyFormat("delete[] *(ptr);", Style);
+
   verifyIndependentOfContext("MACRO(int *i);");
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2372,6 +2372,9 @@
 !PrevToken->MatchingParen)
   return TT_PointerOrReference;
 
+if (PrevToken->endsSequence(tok::r_square, tok::l_square, tok::kw_delete))
+  return TT_UnaryOperator;
+
 if (PrevToken->Tok.isLiteral() ||
 PrevToken->isOneOf(tok::r_paren, tok::r_square, tok::kw_true,
tok::kw_false, tok::r_brace)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132911: Fix annotating when deleting array of pointers

[Jack Huang via Phabricator via cfe-commits]

jackhong12 updated this revision to Diff 456834.
jackhong12 retitled this revision from "[clang-format] Fix annotating when 
deleting array of pointers" to "Fix annotating when deleting array of pointers".
jackhong12 edited the summary of this revision.
jackhong12 added a comment.

Use endsSequence to match the pattern


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132911

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -120,6 +120,17 @@
   Tokens = annotate("int i = int{42} * 2;");
   EXPECT_EQ(Tokens.size(), 11u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("delete[] *ptr;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
+  Tokens = annotate("delete[] **ptr;");
+  EXPECT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::star, TT_UnaryOperator);
+  Tokens = annotate("delete[] *(ptr);");
+  EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10535,6 +10535,11 @@
"} & = {};",
Style);
 
+  Style.PointerAlignment = FormatStyle::PAS_Right;
+  verifyFormat("delete[] *ptr;", Style);
+  verifyFormat("delete[] **ptr;", Style);
+  verifyFormat("delete[] *(ptr);", Style);
+
   verifyIndependentOfContext("MACRO(int *i);");
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2372,6 +2372,9 @@
 !PrevToken->MatchingParen)
   return TT_PointerOrReference;
 
+if (PrevToken->endsSequence(tok::r_square, tok::l_square, tok::kw_delete))
+  return TT_UnaryOperator;
+
 if (PrevToken->Tok.isLiteral() ||
 PrevToken->isOneOf(tok::r_paren, tok::r_square, tok::kw_true,
tok::kw_false, tok::r_brace)) {


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -120,6 +120,17 @@
   Tokens = annotate("int i = int{42} * 2;");
   EXPECT_EQ(Tokens.size(), 11u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("delete[] *ptr;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
+  Tokens = annotate("delete[] **ptr;");
+  EXPECT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::star, TT_UnaryOperator);
+  Tokens = annotate("delete[] *(ptr);");
+  EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10535,6 +10535,11 @@
"} & = {};",
Style);
 
+  Style.PointerAlignment = FormatStyle::PAS_Right;
+  verifyFormat("delete[] *ptr;", Style);
+  verifyFormat("delete[] **ptr;", Style);
+  verifyFormat("delete[] *(ptr);", Style);
+
   verifyIndependentOfContext("MACRO(int *i);");
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2372,6 +2372,9 @@
 !PrevToken->MatchingParen)
   return TT_PointerOrReference;
 
+if (PrevToken->endsSequence(tok::r_square, tok::l_square, tok::kw_delete))
+  return TT_UnaryOperator;
+
 if (PrevToken->Tok.isLiteral() ||
 PrevToken->isOneOf(tok::r_paren, tok::r_square, tok::kw_true,
tok::kw_false, tok::r_brace)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[clang] 146ae41 - Revert "[driver] Additional ignoring of module-map related flags, if modules are disabled"

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2022-08-30T18:31:53-07:00
New Revision: 146ae4138a081a9a10e4901bbec61b629331da8a

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

LOG: Revert "[driver] Additional ignoring of module-map related flags, if 
modules are disabled"

This reverts commit 33162a81d4c93a53ef847d3601b0b03830937d3c.

This change breaks the usage of module maps with modules disabled, such
as for layering checking via `-fmodules-decluse`.

Regression test added.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/modules.m

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 401333a39025f..1fa83e7121c95 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3726,29 +3726,25 @@ static void RenderModulesOptions(Compilation , const 
Driver ,
  options::OPT_fno_modules_validate_input_files_content,
  false))
   CmdArgs.push_back("-fvalidate-ast-input-files-content");
+  }
 
-// -fmodule-name specifies the module that is currently being built (or
-// used for header checking by -fmodule-maps).
-Args.AddLastArg(CmdArgs, options::OPT_fmodule_name_EQ);
-
-// -fmodule-map-file can be used to specify files containing module
-// definitions.
-Args.AddAllArgs(CmdArgs, options::OPT_fmodule_map_file);
-
-// -fbuiltin-module-map can be used to load the clang
-// builtin headers modulemap file.
-if (Args.hasArg(options::OPT_fbuiltin_module_map)) {
-  SmallString<128> BuiltinModuleMap(D.ResourceDir);
-  llvm::sys::path::append(BuiltinModuleMap, "include");
-  llvm::sys::path::append(BuiltinModuleMap, "module.modulemap");
-  if (llvm::sys::fs::exists(BuiltinModuleMap))
-CmdArgs.push_back(
-Args.MakeArgString("-fmodule-map-file=" + BuiltinModuleMap));
-}
-  } else {
-Args.ClaimAllArgs(options::OPT_fmodule_name_EQ);
-Args.ClaimAllArgs(options::OPT_fmodule_map_file);
-Args.ClaimAllArgs(options::OPT_fbuiltin_module_map);
+  // -fmodule-name specifies the module that is currently being built (or
+  // used for header checking by -fmodule-maps).
+  Args.AddLastArg(CmdArgs, options::OPT_fmodule_name_EQ);
+
+  // -fmodule-map-file can be used to specify files containing module
+  // definitions.
+  Args.AddAllArgs(CmdArgs, options::OPT_fmodule_map_file);
+
+  // -fbuiltin-module-map can be used to load the clang
+  // builtin headers modulemap file.
+  if (Args.hasArg(options::OPT_fbuiltin_module_map)) {
+SmallString<128> BuiltinModuleMap(D.ResourceDir);
+llvm::sys::path::append(BuiltinModuleMap, "include");
+llvm::sys::path::append(BuiltinModuleMap, "module.modulemap");
+if (llvm::sys::fs::exists(BuiltinModuleMap))
+  CmdArgs.push_back(
+  Args.MakeArgString("-fmodule-map-file=" + BuiltinModuleMap));
   }
 
   // The -fmodule-file== form specifies the mapping of module

diff  --git a/clang/test/Driver/modules.m b/clang/test/Driver/modules.m
index 7eaea1da5649f..4b67f7c7bec4f 100644
--- a/clang/test/Driver/modules.m
+++ b/clang/test/Driver/modules.m
@@ -75,8 +75,8 @@
 // RUN: %clang -fno-modules -fbuild-session-timestamp=123 -### %s 2>&1 | 
FileCheck -check-prefix=SESSION_FLAG %s
 // SESSION_FLAG-NOT: -fbuild-session-timestamp
 
-// RUN: %clang -fno-modules -fmodules-validate-once-per-build-session 
-fmodules-validate-system-headers -fmodule-map-file=module.modulemap \
-// RUN:   -### %s 2>&1 | FileCheck -check-prefix=IGNORED_FLAGS %s
-// IGNORED_FLAGS-NOT: -fmodules-validate-once-per-build-session
-// IGNORED_FLAGS-NOT: -fmodules-validate-system-headers
-// IGNORED_FLAGS-NOT: -fmodule-map-file
+// RUN: %clang -fno-modules -fmodules-validate-once-per-build-session -### %s 
2>&1 | FileCheck -check-prefix=VALIDATE_ONCE_FLAG %s
+// VALIDATE_ONCE_FLAG-NOT: -fmodules-validate-once-per-build-session
+
+// RUN: %clang -fno-modules -fmodules-validate-system-headers -### %s 2>&1 | 
FileCheck -check-prefix=VALIDATE_SYSTEM_FLAG %s
+// VALIDATE_SYSTEM_FLAG-NOT: -fmodules-validate-system-headers



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

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

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

rsmith added a comment.

This doesn't look right to me -- we still use module maps when modules are 
disabled to enforce layering checking, and when 
`-fmodules-local-submodule-visibility` is enabled but `-fmodules` is disabled 
we'll use them to provide modular semantics without pre-building modules. 
Please can you revert?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132801

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

[PATCH] D132906: [Clang] Fix lambda CheckForDefaultedFunction(...) so that it checks the CXXMethodDecl is a special member function before attempting to call DefineDefaultedFunction(...)

[Shafik Yaghmour 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 rGb9f767884669: [Clang] Fix lambda 
CheckForDefaultedFunction(...) so that it checks the… (authored by shafik).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132906

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/constant-expression-cxx2a.cpp


Index: clang/test/SemaCXX/constant-expression-cxx2a.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1473,3 +1473,13 @@
   }
   static_assert(g()); // expected-error {{constant expression}} expected-note 
{{in call}}
 }
+
+namespace GH57431 {
+class B {
+  virtual int constexpr f() = 0;
+};
+
+class D : B {
+  virtual int constexpr f() = default; // expected-error {{only special member 
functions and comparison operators may be defaulted}}
+};
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -6954,7 +6954,8 @@
 // Define defaulted constexpr virtual functions that override a base class
 // function right away.
 // FIXME: We can defer doing this until the vtable is marked as used.
-if (M->isDefaulted() && M->isConstexpr() && M->size_overridden_methods())
+if (CSM != CXXInvalid && M->isDefaulted() && M->isConstexpr() &&
+M->size_overridden_methods())
   DefineDefaultedFunction(*this, M, M->getLocation());
 
 if (!Incomplete)
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -86,6 +86,9 @@
   `Issue 57387 `_.
 - Fix a crash when emitting a concept-related diagnostic. This fixes
   `Issue 57415 `_.
+- Fix a crash when attempting to default a virtual constexpr non-special member
+  function in a derived class. This fixes
+  `Issue 57431 `_
 
 
 Improvements to Clang's diagnostics


Index: clang/test/SemaCXX/constant-expression-cxx2a.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1473,3 +1473,13 @@
   }
   static_assert(g()); // expected-error {{constant expression}} expected-note {{in call}}
 }
+
+namespace GH57431 {
+class B {
+  virtual int constexpr f() = 0;
+};
+
+class D : B {
+  virtual int constexpr f() = default; // expected-error {{only special member functions and comparison operators may be defaulted}}
+};
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -6954,7 +6954,8 @@
 // Define defaulted constexpr virtual functions that override a base class
 // function right away.
 // FIXME: We can defer doing this until the vtable is marked as used.
-if (M->isDefaulted() && M->isConstexpr() && M->size_overridden_methods())
+if (CSM != CXXInvalid && M->isDefaulted() && M->isConstexpr() &&
+M->size_overridden_methods())
   DefineDefaultedFunction(*this, M, M->getLocation());
 
 if (!Incomplete)
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -86,6 +86,9 @@
   `Issue 57387 `_.
 - Fix a crash when emitting a concept-related diagnostic. This fixes
   `Issue 57415 `_.
+- Fix a crash when attempting to default a virtual constexpr non-special member
+  function in a derived class. This fixes
+  `Issue 57431 `_
 
 
 Improvements to Clang's diagnostics
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] b9f7678 - [Clang] Fix lambda CheckForDefaultedFunction(...) so that it checks the CXXMethodDecl is a special member function before attempting to call DefineDefaultedFunction(...)

[Shafik Yaghmour via cfe-commits]

Author: Shafik Yaghmour
Date: 2022-08-30T18:08:44-07:00
New Revision: b9f767884669db0b5a56d87b0e8733614d8f884d

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

LOG: [Clang] Fix lambda CheckForDefaultedFunction(...) so that it checks the 
CXXMethodDecl is a special member function before attempting to call 
DefineDefaultedFunction(...)

In Sema::CheckCompletedCXXClass(...) It used a lambda CheckForDefaultedFunction
the CXXMethodDecl passed to CheckForDefaultedFunction may not be a special
member function and so before attempting to apply functions that only apply to
special member functions it needs to check. It fails to do this before calling
DefineDefaultedFunction(...). This PR adds that check and test to verify we no
longer crash.

This fixes https://github.com/llvm/llvm-project/issues/57431

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/SemaCXX/constant-expression-cxx2a.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f946018b361c..944d88d23017 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -86,6 +86,9 @@ Bug Fixes
   `Issue 57387 `_.
 - Fix a crash when emitting a concept-related diagnostic. This fixes
   `Issue 57415 `_.
+- Fix a crash when attempting to default a virtual constexpr non-special member
+  function in a derived class. This fixes
+  `Issue 57431 `_
 
 
 Improvements to Clang's diagnostics

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 198de680ae3d..04d06c11e9c0 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -6954,7 +6954,8 @@ void Sema::CheckCompletedCXXClass(Scope *S, CXXRecordDecl 
*Record) {
 // Define defaulted constexpr virtual functions that override a base class
 // function right away.
 // FIXME: We can defer doing this until the vtable is marked as used.
-if (M->isDefaulted() && M->isConstexpr() && M->size_overridden_methods())
+if (CSM != CXXInvalid && M->isDefaulted() && M->isConstexpr() &&
+M->size_overridden_methods())
   DefineDefaultedFunction(*this, M, M->getLocation());
 
 if (!Incomplete)

diff  --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp 
b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
index 6ebec5dc7e30..63ea42995582 100644
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1473,3 +1473,13 @@ namespace PR45879 {
   }
   static_assert(g()); // expected-error {{constant expression}} expected-note 
{{in call}}
 }
+
+namespace GH57431 {
+class B {
+  virtual int constexpr f() = 0;
+};
+
+class D : B {
+  virtual int constexpr f() = default; // expected-error {{only special member 
functions and comparison operators may be defaulted}}
+};
+}



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

[PATCH] D132906: [Clang] Fix lambda CheckForDefaultedFunction(...) so that it checks the CXXMethodDecl is a special member function before attempting to call DefineDefaultedFunction(...)

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:6957
 // FIXME: We can defer doing this until the vtable is marked as used.
-if (M->isDefaulted() && M->isConstexpr() && M->size_overridden_methods())
+if (CSM != CXXInvalid && M->isDefaulted() && M->isConstexpr() &&
+M->size_overridden_methods())

I realized that based on the constraints on the functions called in 
`DefineDefaultedFunction` we also should be checking `isDeleted()` as well but 
based on the way I drafted this PR I should just do a follow-up.


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

https://reviews.llvm.org/D132906

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

[PATCH] D132906: [Clang] Fix lambda CheckForDefaultedFunction(...) so that it checks the CXXMethodDecl is a special member function before attempting to call DefineDefaultedFunction(...)

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 456827.
shafik added a comment.

- Fixed formatting in test
- Added release note


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

https://reviews.llvm.org/D132906

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/constant-expression-cxx2a.cpp


Index: clang/test/SemaCXX/constant-expression-cxx2a.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1473,3 +1473,13 @@
   }
   static_assert(g()); // expected-error {{constant expression}} expected-note 
{{in call}}
 }
+
+namespace GH57431 {
+class B {
+  virtual int constexpr f() = 0;
+};
+
+class D : B {
+  virtual int constexpr f() = default; // expected-error {{only special member 
functions and comparison operators may be defaulted}}
+};
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -6954,7 +6954,8 @@
 // Define defaulted constexpr virtual functions that override a base class
 // function right away.
 // FIXME: We can defer doing this until the vtable is marked as used.
-if (M->isDefaulted() && M->isConstexpr() && M->size_overridden_methods())
+if (CSM != CXXInvalid && M->isDefaulted() && M->isConstexpr() &&
+M->size_overridden_methods())
   DefineDefaultedFunction(*this, M, M->getLocation());
 
 if (!Incomplete)
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -86,6 +86,9 @@
   `Issue 57387 `_.
 - Fix a crash when emitting a concept-related diagnostic. This fixes
   `Issue 57415 `_.
+- Fix a crash when attempting to default a virtual constexpr non-special member
+  function in a derived class. This fixes
+  `Issue 57431 `_
 
 
 Improvements to Clang's diagnostics


Index: clang/test/SemaCXX/constant-expression-cxx2a.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1473,3 +1473,13 @@
   }
   static_assert(g()); // expected-error {{constant expression}} expected-note {{in call}}
 }
+
+namespace GH57431 {
+class B {
+  virtual int constexpr f() = 0;
+};
+
+class D : B {
+  virtual int constexpr f() = default; // expected-error {{only special member functions and comparison operators may be defaulted}}
+};
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -6954,7 +6954,8 @@
 // Define defaulted constexpr virtual functions that override a base class
 // function right away.
 // FIXME: We can defer doing this until the vtable is marked as used.
-if (M->isDefaulted() && M->isConstexpr() && M->size_overridden_methods())
+if (CSM != CXXInvalid && M->isDefaulted() && M->isConstexpr() &&
+M->size_overridden_methods())
   DefineDefaultedFunction(*this, M, M->getLocation());
 
 if (!Incomplete)
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -86,6 +86,9 @@
   `Issue 57387 `_.
 - Fix a crash when emitting a concept-related diagnostic. This fixes
   `Issue 57415 `_.
+- Fix a crash when attempting to default a virtual constexpr non-special member
+  function in a derived class. This fixes
+  `Issue 57431 `_
 
 
 Improvements to Clang's diagnostics
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132911: [clang-format] Fix annotating when deleting array of pointers

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2375-2378
+const FormatToken *Prev = PrevToken;
+if (Prev->is(tok::r_square) && (Prev = Prev->getPreviousNonComment()) &&
+Prev->is(tok::l_square) && (Prev = Prev->getPreviousNonComment()) &&
+Prev->is(tok::kw_delete))

`endsSequence()` is exactly what you want here. ;)



Comment at: clang/unittests/Format/FormatTest.cpp:10538
 
+  Style.PointerAlignment = FormatStyle::PAS_Right;
+  verifyFormat("delete[] *ptr;", Style);

A typo?



Comment at: clang/unittests/Format/FormatTest.cpp:10541
+  verifyFormat("delete[] **ptr;", Style);
+  verifyFormat("delete[] *(ptr);", Style);
+

MyDeveloperDay wrote:
> could you cover the other PointerAlignment case for completeness
IMO, testing only `PAS_Left` would suffice as we just want to ensure that no 
spaces are added after the `*`s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132911

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

[PATCH] D132984: Set HOME for tests that use module cache path

[Colin Cross via Phabricator via cfe-commits]

ccross created this revision.
ccross added a reviewer: pirama.
Herald added a project: All.
ccross requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Getting the default module cache path calls llvm::sys::path::cache_directory,
which calls home_directory, which checks the HOME environment variable
before falling back to getpwuid.  When compiling against musl libc,
which does not support NSS, and running on a machine that doesn't have
the current user in /etc/passwd due to NSS, no home directory can
be found.  Set the HOME environment variable in the tests to avoid
depending on getpwuid.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132984

Files:
  clang/test/Driver/modules-cache-path.m
  clang/test/Modules/driver.c
  clang/test/Unit/lit.cfg.py


Index: clang/test/Unit/lit.cfg.py
===
--- clang/test/Unit/lit.cfg.py
+++ clang/test/Unit/lit.cfg.py
@@ -30,6 +30,9 @@
 if 'TEMP' in os.environ:
 config.environment['TEMP'] = os.environ['TEMP']
 
+if 'HOME' in os.environ:
+config.environment['HOME'] = os.environ['HOME']
+
 # Propagate sanitizer options.
 for var in [
 'ASAN_SYMBOLIZER_PATH',
Index: clang/test/Modules/driver.c
===
--- clang/test/Modules/driver.c
+++ clang/test/Modules/driver.c
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -fimplicit-module-maps %s -### 2>&1 | FileCheck 
-check-prefix CHECK-NO_MODULE_CACHE %s
+// RUN: env HOME=%t.home %clang -fmodules -fimplicit-module-maps %s -### 2>&1 
| FileCheck -check-prefix CHECK-NO_MODULE_CACHE %s
 // RUN: %clang -fmodules -fimplicit-module-maps -fmodules-cache-path=blarg %s 
-### 2>&1 | FileCheck -check-prefix CHECK-WITH_MODULE_CACHE %s
 
 // CHECK-NO_MODULE_CACHE: {{clang.*"-fmodules-cache-path=.*ModuleCache"}}
Index: clang/test/Driver/modules-cache-path.m
===
--- clang/test/Driver/modules-cache-path.m
+++ clang/test/Driver/modules-cache-path.m
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -### %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-DEFAULT
+// RUN: env HOME=%t.home %clang -fmodules -### %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-DEFAULT
 // CHECK-DEFAULT: -fmodules-cache-path={{.*}}clang{{[/\\]+}}ModuleCache
 
 // RUN: env CLANG_MODULE_CACHE_PATH=/dev/null \


Index: clang/test/Unit/lit.cfg.py
===
--- clang/test/Unit/lit.cfg.py
+++ clang/test/Unit/lit.cfg.py
@@ -30,6 +30,9 @@
 if 'TEMP' in os.environ:
 config.environment['TEMP'] = os.environ['TEMP']
 
+if 'HOME' in os.environ:
+config.environment['HOME'] = os.environ['HOME']
+
 # Propagate sanitizer options.
 for var in [
 'ASAN_SYMBOLIZER_PATH',
Index: clang/test/Modules/driver.c
===
--- clang/test/Modules/driver.c
+++ clang/test/Modules/driver.c
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -fimplicit-module-maps %s -### 2>&1 | FileCheck -check-prefix CHECK-NO_MODULE_CACHE %s
+// RUN: env HOME=%t.home %clang -fmodules -fimplicit-module-maps %s -### 2>&1 | FileCheck -check-prefix CHECK-NO_MODULE_CACHE %s
 // RUN: %clang -fmodules -fimplicit-module-maps -fmodules-cache-path=blarg %s -### 2>&1 | FileCheck -check-prefix CHECK-WITH_MODULE_CACHE %s
 
 // CHECK-NO_MODULE_CACHE: {{clang.*"-fmodules-cache-path=.*ModuleCache"}}
Index: clang/test/Driver/modules-cache-path.m
===
--- clang/test/Driver/modules-cache-path.m
+++ clang/test/Driver/modules-cache-path.m
@@ -1,4 +1,4 @@
-// RUN: %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-DEFAULT
+// RUN: env HOME=%t.home %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-DEFAULT
 // CHECK-DEFAULT: -fmodules-cache-path={{.*}}clang{{[/\\]+}}ModuleCache
 
 // RUN: env CLANG_MODULE_CACHE_PATH=/dev/null \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1877d76 - Revert "[clang][deps] Split translation units into individual -cc1 or other commands"

[Ben Langmuir via cfe-commits]

Author: Ben Langmuir
Date: 2022-08-30T15:50:09-07:00
New Revision: 1877d76aa011e6e481630523be5ed2d86d2b10f0

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

LOG: Revert "[clang][deps] Split translation units into individual -cc1 or 
other commands"

Failing on some bots, reverting until I can fix it.

This reverts commit f80a0ea760728e70f70debf744277bc3aa59bc17.

Added: 


Modified: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/diagnostics.c
clang/test/ClangScanDeps/header-search-pruning-transitive.c
clang/test/ClangScanDeps/modules-context-hash-ignore-macros.c
clang/test/ClangScanDeps/modules-context-hash-outputs.c
clang/test/ClangScanDeps/modules-context-hash-warnings.c
clang/test/ClangScanDeps/modules-context-hash.c
clang/test/ClangScanDeps/modules-dep-args.c
clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
clang/test/ClangScanDeps/modules-full.cpp
clang/test/ClangScanDeps/modules-implicit-dot-private.m
clang/test/ClangScanDeps/modules-incomplete-umbrella.c
clang/test/ClangScanDeps/modules-inferred.m
clang/test/ClangScanDeps/modules-no-undeclared-includes.c
clang/test/ClangScanDeps/modules-pch-common-submodule.c
clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
clang/test/ClangScanDeps/modules-pch.c
clang/test/ClangScanDeps/removed-args.c
clang/tools/clang-scan-deps/ClangScanDeps.cpp
clang/utils/module-deps-to-rsp.py

Removed: 
clang/test/ClangScanDeps/deprecated-driver-api.c
clang/test/ClangScanDeps/multiple-commands.c



diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index c0d273297f18f..cc3f330828a39 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -49,16 +49,8 @@ struct FullDependencies {
   /// determined that the 
diff erences are benign for this compilation.
   std::vector ClangModuleDeps;
 
-  /// The sequence of commands required to build the translation unit. Commands
-  /// should be executed in order.
-  ///
-  /// FIXME: If we add support for multi-arch builds in clang-scan-deps, we
-  /// should make the dependencies between commands explicit to enable parallel
-  /// builds of each architecture.
-  std::vector Commands;
-
-  /// Deprecated driver command-line. This will be removed in a future version.
-  std::vector DriverCommandLine;
+  /// The command line of the TU (excluding the compiler executable).
+  std::vector CommandLine;
 };
 
 struct FullDependenciesResult {
@@ -107,12 +99,6 @@ class DependencyScanningTool {
   LookupModuleOutputCallback LookupModuleOutput,
   llvm::Optional ModuleName = None);
 
-  llvm::Expected 
getFullDependenciesLegacyDriverCommand(
-  const std::vector , StringRef CWD,
-  const llvm::StringSet<> ,
-  LookupModuleOutputCallback LookupModuleOutput,
-  llvm::Optional ModuleName = None);
-
 private:
   DependencyScanningWorker Worker;
 };
@@ -125,10 +111,6 @@ class FullDependencyConsumer : public DependencyConsumer {
   : AlreadySeen(AlreadySeen), LookupModuleOutput(LookupModuleOutput),
 EagerLoadModules(EagerLoadModules) {}
 
-  void handleBuildCommand(Command Cmd) override {
-Commands.push_back(std::move(Cmd));
-  }
-
   void handleDependencyOutputOpts(const DependencyOutputOptions &) override {}
 
   void handleFileDependency(StringRef File) override {
@@ -152,17 +134,14 @@ class FullDependencyConsumer : public DependencyConsumer {
 return LookupModuleOutput(ID, Kind);
   }
 
-  FullDependenciesResult getFullDependenciesLegacyDriverCommand(
+  FullDependenciesResult getFullDependencies(
   const std::vector ) const;
 
-  FullDependenciesResult takeFullDependencies();
-
 private:
   std::vector Dependencies;
   std::vector PrebuiltModuleDeps;
   llvm::MapVector>
   ClangModuleDeps;
-  std::vector Commands;
   std::string ContextHash;
   std::vector OutputPaths;
   const llvm::StringSet<> 

diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 221906f9f9382..b7015ca0ca43c 100644
--- 

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

[Ben Langmuir 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 rGf80a0ea76072: [clang][deps] Split translation units into 
individual -cc1 or other commands (authored by benlangmuir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132405

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/deprecated-driver-api.c
  clang/test/ClangScanDeps/diagnostics.c
  clang/test/ClangScanDeps/header-search-pruning-transitive.c
  clang/test/ClangScanDeps/modules-context-hash-ignore-macros.c
  clang/test/ClangScanDeps/modules-context-hash-outputs.c
  clang/test/ClangScanDeps/modules-context-hash-warnings.c
  clang/test/ClangScanDeps/modules-context-hash.c
  clang/test/ClangScanDeps/modules-dep-args.c
  clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
  clang/test/ClangScanDeps/modules-full.cpp
  clang/test/ClangScanDeps/modules-implicit-dot-private.m
  clang/test/ClangScanDeps/modules-incomplete-umbrella.c
  clang/test/ClangScanDeps/modules-inferred.m
  clang/test/ClangScanDeps/modules-no-undeclared-includes.c
  clang/test/ClangScanDeps/modules-pch-common-submodule.c
  clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
  clang/test/ClangScanDeps/modules-pch.c
  clang/test/ClangScanDeps/multiple-commands.c
  clang/test/ClangScanDeps/removed-args.c
  clang/tools/clang-scan-deps/ClangScanDeps.cpp
  clang/utils/module-deps-to-rsp.py

Index: clang/utils/module-deps-to-rsp.py
===
--- clang/utils/module-deps-to-rsp.py
+++ clang/utils/module-deps-to-rsp.py
@@ -48,6 +48,9 @@
   type=str)
   action.add_argument("--tu-index", help="The index of the translation unit to get arguments for",
   type=int)
+  parser.add_argument("--tu-cmd-index",
+  help="The index of the command within the translation unit (default=0)",
+  type=int, default=0)
   args = parser.parse_args()
 
   full_deps = parseFullDeps(json.load(open(args.full_deps_file, 'r')))
@@ -58,7 +61,8 @@
 if args.module_name:
   cmd = findModule(args.module_name, full_deps)['command-line']
 elif args.tu_index != None:
-  cmd = full_deps.translation_units[args.tu_index]['command-line']
+  tu = full_deps.translation_units[args.tu_index]
+  cmd = tu['commands'][args.tu_cmd_index]['command-line']
 
 print(" ".join(map(quote, cmd)))
   except:
Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -182,6 +182,11 @@
 llvm::cl::desc("The names of dependency targets for the dependency file"),
 llvm::cl::cat(DependencyScannerCategory));
 
+llvm::cl::opt DeprecatedDriverCommand(
+"deprecated-driver-command", llvm::cl::Optional,
+llvm::cl::desc("use a single driver command to build the tu (deprecated)"),
+llvm::cl::cat(DependencyScannerCategory));
+
 enum ResourceDirRecipeKind {
   RDRK_ModifyCompilerPath,
   RDRK_InvokeCompiler,
@@ -256,7 +261,7 @@
 public:
   void mergeDeps(StringRef Input, FullDependenciesResult FDR,
  size_t InputIndex) {
-const FullDependencies  = FDR.FullDeps;
+FullDependencies  = FDR.FullDeps;
 
 InputDeps ID;
 ID.FileName = std::string(Input);
@@ -274,7 +279,8 @@
   Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
 }
 
-ID.CommandLine = FD.CommandLine;
+ID.DriverCommandLine = std::move(FD.DriverCommandLine);
+ID.Commands = std::move(FD.Commands);
 Inputs.push_back(std::move(ID));
   }
 
@@ -311,14 +317,33 @@
 
 Array TUs;
 for (auto & : Inputs) {
-  Object O{
-  {"input-file", I.FileName},
-  {"clang-context-hash", I.ContextHash},
-  {"file-deps", I.FileDeps},
-  {"clang-module-deps", toJSONSorted(I.ModuleDeps)},
-  {"command-line", I.CommandLine},
-  };
-  TUs.push_back(std::move(O));
+  Array Commands;
+  if (I.DriverCommandLine.empty()) {
+for (const auto  : I.Commands) {
+  Object O{
+  {"input-file", I.FileName},
+  {"clang-context-hash", I.ContextHash},
+  {"file-deps", I.FileDeps},
+  {"clang-module-deps", toJSONSorted(I.ModuleDeps)},
+  {"executable", Cmd.Executable},
+  {"command-line", Cmd.Arguments},
+ 

[clang] f80a0ea - [clang][deps] Split translation units into individual -cc1 or other commands

[Ben Langmuir via cfe-commits]

Author: Ben Langmuir
Date: 2022-08-30T15:23:19-07:00
New Revision: f80a0ea760728e70f70debf744277bc3aa59bc17

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

LOG: [clang][deps] Split translation units into individual -cc1 or other 
commands

Instead of trying to "fix" the original driver invocation by appending
arguments to it, split it into multiple commands, and for each -cc1
command use a CompilerInvocation to give precise control over the
invocation.

This change should make it easier to (in the future) canonicalize the
command-line (e.g. to improve hits in something like ccache), apply
optimizations, or start supporting multi-arch builds, which would
require different modules for each arch.

In the long run it may make sense to treat the TU commands as a
dependency graph, each with their own dependencies on modules or earlier
TU commands, but for now they are simply a list that is executed in
order, and the dependencies are simply duplicated. Since we currently
only support single-arch builds, there is no parallelism available in
the execution.

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

Added: 
clang/test/ClangScanDeps/deprecated-driver-api.c
clang/test/ClangScanDeps/multiple-commands.c

Modified: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/diagnostics.c
clang/test/ClangScanDeps/header-search-pruning-transitive.c
clang/test/ClangScanDeps/modules-context-hash-ignore-macros.c
clang/test/ClangScanDeps/modules-context-hash-outputs.c
clang/test/ClangScanDeps/modules-context-hash-warnings.c
clang/test/ClangScanDeps/modules-context-hash.c
clang/test/ClangScanDeps/modules-dep-args.c
clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
clang/test/ClangScanDeps/modules-full.cpp
clang/test/ClangScanDeps/modules-implicit-dot-private.m
clang/test/ClangScanDeps/modules-incomplete-umbrella.c
clang/test/ClangScanDeps/modules-inferred.m
clang/test/ClangScanDeps/modules-no-undeclared-includes.c
clang/test/ClangScanDeps/modules-pch-common-submodule.c
clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
clang/test/ClangScanDeps/modules-pch.c
clang/test/ClangScanDeps/removed-args.c
clang/tools/clang-scan-deps/ClangScanDeps.cpp
clang/utils/module-deps-to-rsp.py

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index cc3f330828a39..c0d273297f18f 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -49,8 +49,16 @@ struct FullDependencies {
   /// determined that the 
diff erences are benign for this compilation.
   std::vector ClangModuleDeps;
 
-  /// The command line of the TU (excluding the compiler executable).
-  std::vector CommandLine;
+  /// The sequence of commands required to build the translation unit. Commands
+  /// should be executed in order.
+  ///
+  /// FIXME: If we add support for multi-arch builds in clang-scan-deps, we
+  /// should make the dependencies between commands explicit to enable parallel
+  /// builds of each architecture.
+  std::vector Commands;
+
+  /// Deprecated driver command-line. This will be removed in a future version.
+  std::vector DriverCommandLine;
 };
 
 struct FullDependenciesResult {
@@ -99,6 +107,12 @@ class DependencyScanningTool {
   LookupModuleOutputCallback LookupModuleOutput,
   llvm::Optional ModuleName = None);
 
+  llvm::Expected 
getFullDependenciesLegacyDriverCommand(
+  const std::vector , StringRef CWD,
+  const llvm::StringSet<> ,
+  LookupModuleOutputCallback LookupModuleOutput,
+  llvm::Optional ModuleName = None);
+
 private:
   DependencyScanningWorker Worker;
 };
@@ -111,6 +125,10 @@ class FullDependencyConsumer : public DependencyConsumer {
   : AlreadySeen(AlreadySeen), LookupModuleOutput(LookupModuleOutput),
 EagerLoadModules(EagerLoadModules) {}
 
+  void handleBuildCommand(Command Cmd) override {
+Commands.push_back(std::move(Cmd));
+  }
+
   void handleDependencyOutputOpts(const DependencyOutputOptions &) override {}
 
   void handleFileDependency(StringRef File) override {
@@ -134,14 +152,17 @@ class 

[PATCH] D132977: [HLSL] Call global constructors inside entry

[Xiang Li via Phabricator via cfe-commits]

python3kgae added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:151
+  llvm::Module  = CGM.getModule();
+  const auto *GlobalCtors = M.getNamedGlobal("llvm.global_ctors");
+  if (!GlobalCtors)

beanz wrote:
> python3kgae wrote:
> > Don't need to generate CtorCalls for lib profile in clang codeGen.
> > Have to do this when linking anyway.
> Are you sure? The global constructors contain the `createHandle` calls for 
> resources, and DXC does currently generate those inside shader entries for 
> lib shaders.
> 
> I'm sure we're not generating those _correctly_ with this change, but I think 
> we still need the constructor calls inside any function annotated with the 
> `[shader(...)]` attribute.
I saw dxc did call ctor for all entries. Maybe to match the behavior, we need 
to do the same thing.
But I don't understand why we must do that except to match what dxc does.
Maybe we can also have a test for lib profile to make understanding things 
easier?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132977

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

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D132952#3759731 , @efriedma wrote:

> As a practical matter, there isn't any reason to force variably modified 
> parameters to make a function variably modified.  The types of parameters 
> aren't visible in the caller of a function; we only check the compatibility 
> for calls.
>
> Trying to treat `void (*)(int, int *)` as variably modified would have 
> additional complications, in that clang would internally need to have two 
> canonical types for `void (*)(int, int *)`: one variably modified, one not 
> variably modified.

I have been thinking about this problem for different reasons.

I wonder if the right solution here is to just change AdjustedType so that it 
desugars to the OriginalType instead of the AdjustedType, but keep the latter 
as the canonical type.
And not bother storing a separate QualType fot the AdjustedType.

That way, after a decay, the as-written type could be VM, but not the canonical 
type.

Similarly with `AttributedType` vis EquivalentType and ModifiedType.

We would lose though the nice property that a single step desugar always 
produces the same type.

I wonder how radical a change this would be for the users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132952

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

[PATCH] D132907: [msan] Add more specific messages for use-after-destroy

[Kirill Stoimenov via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGClass.cpp:1773
   // function
-  EmitSanitizerDtorCallback(CGF, VTablePtr, PoisonSize);
+  EmitSanitizerDtorCallback(CGF, "__sanitizer_dtor_callback_vptr",
+VTablePtr);

Should this be a constant? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132907

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

[PATCH] D132977: [HLSL] Call global constructors inside entry

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:151
+  llvm::Module  = CGM.getModule();
+  const auto *GlobalCtors = M.getNamedGlobal("llvm.global_ctors");
+  if (!GlobalCtors)

python3kgae wrote:
> Don't need to generate CtorCalls for lib profile in clang codeGen.
> Have to do this when linking anyway.
Are you sure? The global constructors contain the `createHandle` calls for 
resources, and DXC does currently generate those inside shader entries for lib 
shaders.

I'm sure we're not generating those _correctly_ with this change, but I think 
we still need the constructor calls inside any function annotated with the 
`[shader(...)]` attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132977

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

[clang] 260fb2b - [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin module

[Ian Anderson via cfe-commits]

Author: Ian Anderson
Date: 2022-08-30T14:57:15-07:00
New Revision: 260fb2bc3f79019cae4e182a64c8752d3d25049e

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

LOG: [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin 
module

The Darwin module has specified [no_undeclared_includes] for at least five 
years now, there's no need to hard code it in the compiler.

Reviewed By: ributzka, Bigcheese

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

Added: 


Modified: 
clang/lib/Lex/ModuleMap.cpp

Removed: 




diff  --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 3698a8c932d72..87a90bc3e704f 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -2026,8 +2026,7 @@ void ModuleMapParser::parseModuleDecl() {
 ActiveModule->IsSystem = true;
   if (Attrs.IsExternC)
 ActiveModule->IsExternC = true;
-  if (Attrs.NoUndeclaredIncludes ||
-  (!ActiveModule->Parent && ModuleName == "Darwin"))
+  if (Attrs.NoUndeclaredIncludes)
 ActiveModule->NoUndeclaredIncludes = true;
   ActiveModule->Directory = Directory;
 



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

[PATCH] D132971: [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin module

[Ian Anderson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG260fb2bc3f79: [clang][modules] Dont hard code 
[no_undeclared_includes] for the Darwin module (authored by iana).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132971

Files:
  clang/lib/Lex/ModuleMap.cpp


Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -2026,8 +2026,7 @@
 ActiveModule->IsSystem = true;
   if (Attrs.IsExternC)
 ActiveModule->IsExternC = true;
-  if (Attrs.NoUndeclaredIncludes ||
-  (!ActiveModule->Parent && ModuleName == "Darwin"))
+  if (Attrs.NoUndeclaredIncludes)
 ActiveModule->NoUndeclaredIncludes = true;
   ActiveModule->Directory = Directory;
 


Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -2026,8 +2026,7 @@
 ActiveModule->IsSystem = true;
   if (Attrs.IsExternC)
 ActiveModule->IsExternC = true;
-  if (Attrs.NoUndeclaredIncludes ||
-  (!ActiveModule->Parent && ModuleName == "Darwin"))
+  if (Attrs.NoUndeclaredIncludes)
 ActiveModule->NoUndeclaredIncludes = true;
   ActiveModule->Directory = Directory;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132977: [HLSL] Call global constructors inside entry

[Xiang Li via Phabricator via cfe-commits]

python3kgae added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:151
+  llvm::Module  = CGM.getModule();
+  const auto *GlobalCtors = M.getNamedGlobal("llvm.global_ctors");
+  if (!GlobalCtors)

Don't need to generate CtorCalls for lib profile in clang codeGen.
Have to do this when linking anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132977

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

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

[Ben Langmuir via Phabricator via cfe-commits]

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



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:295
+if (MDC)
+  MDC->applyDiscoveredDependencies(CI);
+LastCC1Arguments = CI.getCC1CommandLine();

jansvoboda11 wrote:
> I'm not a fan of storing `MDC` as a member, but I don't see any clearly 
> better alternatives. One option would be to pass `CompilerInvocation` 
> out-parameter to `ModuleDepCollector`'s constructor and relying on the 
> collector to apply discovered dependencies, but that's not super obvious.
We are applying changes to multiple CompilerInvocations (the scanned one, but 
also any later ones that are downstream of it), so it's less obvious to me how 
to do that here.  We would need to pass in all of the invocations, but 
currently only one of them is in-memory at a time.  I'm open to other ideas 
here, but I haven't come up with any great ideas.  Maybe there's a refactoring 
of MDC that splits the scanning state from the state necessary to apply changes 
to the CI so we would only expose the minimum information needed, but I expect 
that would be an invasive change so prefer not to attempt it in this patch.


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

https://reviews.llvm.org/D132405

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

[PATCH] D132977: [HLSL] Call global constructors inside entry

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: aaron.ballman, bogner, python3kgae, pow2clk, tex3d, 
eli.friedman.
Herald added a subscriber: Anastasia.
Herald added a project: All.
beanz requested review of this revision.
Herald added a project: clang.

HLSL doesn't have a runtime loader model that supports global
construction by a loader or runtime initializer. To allow us to leverage
global constructors with minimal code generation impact we put calls to
the global constructors inside the generated entry function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132977

Files:
  clang/docs/HLSL/EntryFunctions.rst
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/test/CodeGenHLSL/GlobalConstructors.hlsl

Index: clang/test/CodeGenHLSL/GlobalConstructors.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/GlobalConstructors.hlsl
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -S -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s
+
+RWBuffer Buffer;
+
+[numthreads(1,1,1)]
+void main(unsigned GI : SV_GroupIndex) {}
+
+//CHECK:  define void @main()
+//CHECK-NEXT: entry:
+//CHECK-NEXT:   call void @_GLOBAL__sub_I_GlobalConstructors.hlsl()
+//CHECK-NEXT:   %0 = call i32 @llvm.dx.flattened.thread.id.in.group()
+//CHECK-NEXT:   call void @"?main@@YAXI@Z"(i32 %0)
+//CHECK-NEXT:   ret void
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -46,6 +46,7 @@
   virtual ~CGHLSLRuntime() {}
 
   void annotateHLSLResource(const VarDecl *D, llvm::GlobalVariable *GV);
+  void generateGlobalCtorCalls();
 
   void finishCodeGen();
 
Index: clang/lib/CodeGen/CGHLSLRuntime.cpp
===
--- clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -54,6 +54,8 @@
   Triple T(M.getTargetTriple());
   if (T.getArch() == Triple::ArchType::dxil)
 addDxilValVersion(TargetOpts.DxilValidatorVersion, M);
+
+  generateGlobalCtorCalls();
 }
 
 void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, GlobalVariable *GV) {
@@ -143,3 +145,40 @@
   // FIXME: Handle codegen for return type semantics.
   B.CreateRetVoid();
 }
+
+void CGHLSLRuntime::generateGlobalCtorCalls() {
+  llvm::Module  = CGM.getModule();
+  const auto *GlobalCtors = M.getNamedGlobal("llvm.global_ctors");
+  if (!GlobalCtors)
+return;
+  const auto *CA = dyn_cast(GlobalCtors->getInitializer());
+  if (!CA)
+return;
+  // The global_ctor array elements are a struct [Priority, Fn *, COMDat].
+  // HLSL neither supports priorities or COMDat values, so we will check those
+  // in an assert but not handle them.
+
+  llvm::SmallVector CtorFns;
+  for (const auto  : CA->operands()) {
+if (isa(Ctor))
+  continue;
+ConstantStruct *CS = cast(Ctor);
+
+assert(cast(CS->getOperand(0))->getValue() == 65535 &&
+   "HLSL doesn't support setting priority for global ctors.");
+assert(isa(CS->getOperand(2)) &&
+   "HLSL doesn't support COMDat for global ctors.");
+CtorFns.push_back(cast(CS->getOperand(1)));
+  }
+
+  // Insert a call to the global constructor at the beginning of the entry block
+  // to externally exported functions. This is a bit of a hack, but HLSL allows
+  // global constructors, but doesn't support driver initialization of globals.
+  for (auto  : M.functions()) {
+if (!F.hasFnAttribute("hlsl.shader"))
+  continue;
+IRBuilder<> B((), F.getEntryBlock().begin());
+for (auto *Fn : CtorFns)
+  B.CreateCall(FunctionCallee(Fn));
+  }
+}
Index: clang/docs/HLSL/EntryFunctions.rst
===
--- clang/docs/HLSL/EntryFunctions.rst
+++ clang/docs/HLSL/EntryFunctions.rst
@@ -37,10 +37,16 @@
 
 The actual exported entry function which can be called by the GPU driver is a
 ``void(void)`` function that isn't name mangled. In code generation we generate
-the unmangled entry function, instantiations of the parameters with their
-semantic values populated, and a call to the user-defined function. After the
-call instruction the return value (if any) is saved using a target-appropriate
-intrinsic for storing outputs (for DirectX, the ``llvm.dx.store.output``).
+the unmangled entry function to serve as the actual shader entry. The shader
+entry function is annotated with the ``hlsl.shader`` function attribute
+identifying the entry's pipeline stage.
+
+The body of the unmangled entry function contains first a call to execute global
+constructors, then instantiations of the user-defined entry parameters with
+their semantic values populated, and a call to the user-defined function.
+After the call instruction the return value (if any) is saved using a
+target-appropriate intrinsic for 

[PATCH] D132713: [clang-tidy] Skip union-like classes in use-equals-default

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov updated this revision to Diff 456800.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132713

Files:
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
@@ -42,6 +42,17 @@
   NE Field;
 };
 
+// Skip structs/classes containing anonymous unions.
+struct SU {
+  SU() {}
+  // CHECK-FIXES: SU() {}
+  ~SU() {}
+  // CHECK-FIXES: ~SU() {}
+  union {
+NE Field;
+  };
+};
+
 // Initializer or arguments.
 class IA {
 public:
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
@@ -44,6 +44,20 @@
   IL Field;
 };
 
+// Skip structs/classes containing anonymous unions.
+struct SU {
+  SU(const SU ) : Field(Other.Field) {}
+  // CHECK-FIXES: SU(const SU ) :
+  SU =(const SU ) {
+Field = Other.Field;
+return *this;
+  }
+  // CHECK-FIXES: SU =(const SU ) {
+  union {
+IL Field;
+  };
+};
+
 // Wrong type.
 struct WT {
   WT(const IL ) {}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -134,8 +134,8 @@
 
 - Improved `modernize-use-equals-default `_ check.
 
-  The check now skips unions since in this case a default constructor with empty body
-  is not equivalent to the explicitly defaulted one.
+  The check now skips unions/union-like classes since in this case a default constructor
+  with empty body is not equivalent to the explicitly defaulted one.
 
 Removed checks
 ^^
Index: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -217,17 +217,20 @@
 }
 
 void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) {
-  // Skip unions since constructors with empty bodies behave differently
-  // in comparison with structs/classes.
+  // Skip unions/union-like classes since their constructors behave differently
+  // when defaulted vs. empty.
+  auto IsUnionLikeClass = recordDecl(
+  anyOf(isUnion(),
+has(fieldDecl(isImplicit(), hasType(cxxRecordDecl(isUnion()));
 
   // Destructor.
-  Finder->addMatcher(cxxDestructorDecl(unless(hasParent(recordDecl(isUnion(,
-   isDefinition())
- .bind(SpecialFunction),
- this);
+  Finder->addMatcher(
+  cxxDestructorDecl(unless(hasParent(IsUnionLikeClass)), isDefinition())
+  .bind(SpecialFunction),
+  this);
   Finder->addMatcher(
   cxxConstructorDecl(
-  unless(hasParent(recordDecl(isUnion(, isDefinition(),
+  unless(hasParent(IsUnionLikeClass)), isDefinition(),
   anyOf(
   // Default constructor.
   allOf(unless(hasAnyConstructorInitializer(isWritten())),
@@ -242,7 +245,7 @@
   this);
   // Copy-assignment operator.
   Finder->addMatcher(
-  cxxMethodDecl(unless(hasParent(recordDecl(isUnion(, isDefinition(),
+  cxxMethodDecl(unless(hasParent(IsUnionLikeClass)), isDefinition(),
 isCopyAssignmentOperator(),
 // isCopyAssignmentOperator() allows the parameter to be
 // passed by value, and in this case it cannot be
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132975: [clang][BOLT] Add clang-bolt target (WIP)

[Amir Ayupov via Phabricator via cfe-commits]

Amir updated this revision to Diff 456799.
Amir added a comment.

CMAKE_CURRENT_BINARY_DIR already contains bin/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132975

Files:
  clang/CMakeLists.txt
  clang/cmake/caches/BOLT.cmake


Index: clang/cmake/caches/BOLT.cmake
===
--- /dev/null
+++ clang/cmake/caches/BOLT.cmake
@@ -0,0 +1,12 @@
+set(CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(CLANG_BOLT_INSTRUMENT ON CACHE BOOL "")
+set(CMAKE_EXE_LINKER_FLAGS "-Wl,--emit-relocs,-znow" CACHE STRING "")
+
+set(LLVM_ENABLE_PROJECTS "bolt;clang;lld" CACHE STRING "")
+set(LLVM_TARGETS_TO_BUILD Native CACHE STRING "")
+
+# Disable function splitting enabled by default in GCC8+
+if("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU")
+  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-reorder-blocks-and-partition")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-reorder-blocks-and-partition")
+endif()
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -878,6 +878,56 @@
   endforeach()
 endif()
 
+if (CLANG_BOLT_INSTRUMENT)
+  find_program(LLVM_BOLT llvm-bolt)
+  find_program(MERGE_FDATA merge-fdata)
+
+  # Instrument clang with BOLT
+  add_custom_target(clang-instrumented
+DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/clang.bolt.inst
+  )
+  add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/clang.bolt.inst
+DEPENDS clang
+COMMAND ${LLVM_BOLT} ${CMAKE_BINARY_DIR}/clang -o
+  ${CMAKE_CURRENT_BINARY_DIR}/clang.bolt.inst
+  -instrument --instrumentation-file-append-pid
+  --instrumentation-file=${CMAKE_CURRENT_BINARY_DIR}/prof.fdata
+COMMENT "Instrumenting clang binary with BOLT"
+)
+  # Make a symlink from clang.bolt.inst to clang++.bolt.inst
+  add_clang_symlink(${CMAKE_CURRENT_BINARY_DIR}/clang++.bolt.inst
+${CMAKE_CURRENT_BINARY_DIR}/clang.bolt.inst)
+
+  # Configure and build Clang with instrumented Clang to collect the profile
+  include(ExternalProject)
+  ExternalProject_Add(bolt-instrumentation-profile
+DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/clang.bolt.inst
+PREFIX bolt-instrumentation-profile
+SOURCE_DIR ${CMAKE_SOURCE_DIR}
+CMAKE_ARGS
+# We shouldn't need to set this here, but INSTALL_DIR doesn't
+# seem to work, so instead I'm passing this through
+-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
+
-DCMAKE_CXX_COMPILER=${CMAKE_CURRENT_BINARY_DIR}/clang++.bolt.inst
+-DCMAKE_C_COMPILER=${CMAKE_CURRENT_BINARY_DIR}/clang.bolt.inst
+STEP_TARGETS configure build install
+  )
+  # Merge profiles into one using merge-fdata
+  add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/prof.fdata
+COMMAND ${MERGE_FDATA} ${CMAKE_CURRENT_BINARY_DIR}/prof.fdata.*
+  -o ${CMAKE_CURRENT_BINARY_DIR}/prof.fdata
+  )
+  # Optimize original (pre-bolt) Clang using the collected profile
+  add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/clang.bolt
+DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/prof.fdata clang
+COMMAND ${LLVM_BOLT} ${CMAKE_BINARY_DIR}/clang
+-o ${CMAKE_CURRENT_BINARY_DIR}/clang.bolt -data 
${CMAKE_CURRENT_BINARY_DIR}/prof.fdata
+  )
+  add_custom_target(clang-bolt
+DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/clang.bolt
+)
+endif()
+
 if (LLVM_ADD_NATIVE_VISUALIZERS_TO_SOLUTION)
   add_subdirectory(utils/ClangVisualizers)
 endif()


Index: clang/cmake/caches/BOLT.cmake
===
--- /dev/null
+++ clang/cmake/caches/BOLT.cmake
@@ -0,0 +1,12 @@
+set(CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(CLANG_BOLT_INSTRUMENT ON CACHE BOOL "")
+set(CMAKE_EXE_LINKER_FLAGS "-Wl,--emit-relocs,-znow" CACHE STRING "")
+
+set(LLVM_ENABLE_PROJECTS "bolt;clang;lld" CACHE STRING "")
+set(LLVM_TARGETS_TO_BUILD Native CACHE STRING "")
+
+# Disable function splitting enabled by default in GCC8+
+if("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU")
+  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-reorder-blocks-and-partition")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-reorder-blocks-and-partition")
+endif()
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -878,6 +878,56 @@
   endforeach()
 endif()
 
+if (CLANG_BOLT_INSTRUMENT)
+  find_program(LLVM_BOLT llvm-bolt)
+  find_program(MERGE_FDATA merge-fdata)
+
+  # Instrument clang with BOLT
+  add_custom_target(clang-instrumented
+DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/clang.bolt.inst
+  )
+  add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/clang.bolt.inst
+DEPENDS clang
+COMMAND ${LLVM_BOLT} ${CMAKE_BINARY_DIR}/clang -o
+  ${CMAKE_CURRENT_BINARY_DIR}/clang.bolt.inst
+  -instrument --instrumentation-file-append-pid
+  

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

As a practical matter, there isn't any reason to force variably modified 
parameters to make a function variably modified.  The types of parameters 
aren't visible in the caller of a function; we only check the compatibility for 
calls.

Trying to treat `void (*)(int, int *)` as variably modified would have 
additional complications, in that clang would internally need to have two 
canonical types for `void (*)(int, int *)`: one variably modified, one not 
variably modified.

So if there's a disagreement between clang and the standard, I'd rather just 
try to fix the standard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132952

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

[PATCH] D132975: [clang][BOLT] Add clangbolt target (WIP)

[Amir Ayupov via Phabricator via cfe-commits]

Amir created this revision.
Amir added a reviewer: bolt.
Herald added subscribers: treapster, wenlei, mgorny.
Herald added a project: All.
Amir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch adds `CLANG_BOLT_INSTRUMENT` option that applies BOLT instrumentation
to Clang, performs a bootstrap build with the resulting Clang, merges resulting
fdata files into a single profile file, and uses it to perform BOLT optimization
on the original Clang binary.

The intended use of the functionality is through BOLT CMake cache file, similar
to PGO 2-stage build:

  cmake /llvm -C /clang/cmake/caches/BOLT.cmake
  ninja clang-bolt


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132975

Files:
  clang/CMakeLists.txt
  clang/cmake/caches/BOLT.cmake


Index: clang/cmake/caches/BOLT.cmake
===
--- /dev/null
+++ clang/cmake/caches/BOLT.cmake
@@ -0,0 +1,12 @@
+set(CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(CLANG_BOLT_INSTRUMENT ON CACHE BOOL "")
+set(CMAKE_EXE_LINKER_FLAGS "-Wl,--emit-relocs,-znow" CACHE STRING "")
+
+set(LLVM_ENABLE_PROJECTS "bolt;clang;lld" CACHE STRING "")
+set(LLVM_TARGETS_TO_BUILD Native CACHE STRING "")
+
+# Disable function splitting enabled by default in GCC8+
+if("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU")
+  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-reorder-blocks-and-partition")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-reorder-blocks-and-partition")
+endif()
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -878,6 +878,57 @@
   endforeach()
 endif()
 
+if (CLANG_BOLT_INSTRUMENT)
+  find_program(LLVM_BOLT llvm-bolt)
+  find_program(MERGE_FDATA merge-fdata)
+
+  # Instrument clang with BOLT
+  add_custom_target(clang-instrumented
+DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/bin/clang.bolt.inst
+  )
+  add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/bin/clang.bolt.inst
+DEPENDS clang
+COMMAND ${LLVM_BOLT} ${CMAKE_BINARY_DIR}/bin/clang -o
+  ${CMAKE_CURRENT_BINARY_DIR}/bin/clang.bolt.inst
+  -instrument --instrumentation-file-append-pid
+  --instrumentation-file=${CMAKE_CURRENT_BINARY_DIR}/bin/prof.fdata
+COMMENT "Instrumenting clang binary with BOLT"
+)
+  # Make a symlink from clang.bolt.inst to clang++.bolt.inst
+  add_clang_symlink(${CMAKE_CURRENT_BINARY_DIR}/bin/clang++.bolt.inst
+${CMAKE_CURRENT_BINARY_DIR}/bin/clang.bolt.inst)
+
+  # Configure and build Clang with instrumented Clang to collect the profile
+  include(ExternalProject)
+  ExternalProject_Add(bolt-instrumentation-profile
+DEPENDS clang
+PREFIX bolt-instrumentation-profile
+SOURCE_DIR ${CMAKE_SOURCE_DIR}
+CMAKE_ARGS
+# We shouldn't need to set this here, but INSTALL_DIR doesn't
+# seem to work, so instead I'm passing this through
+-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
+
-DCMAKE_CXX_COMPILER=${CMAKE_CURRENT_BINARY_DIR}/bin/clang++.bolt.inst
+
-DCMAKE_C_COMPILER=${CMAKE_CURRENT_BINARY_DIR}/bin/clang.bolt.inst
+STEP_TARGETS configure build install
+  )
+  # Merge profiles into one using merge-fdata
+  add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/bin/prof.fdata
+COMMAND ${MERGE_FDATA} ${CMAKE_CURRENT_BINARY_DIR}/bin/prof.fdata.*
+  -o ${CMAKE_CURRENT_BINARY_DIR}/bin/prof.fdata
+  )
+  # Optimize original (pre-bolt) Clang using the collected profile
+  add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/bin/clang.bolt
+DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/bin/prof.fdata clang
+COMMAND ${CMAKE_BINARY_DIR}/bin/llvm-bolt ${CMAKE_BINARY_DIR}/bin/clang
+-o ${CMAKE_BINARY_DIR}/bin/clang.bolt -fdata
+${CMAKE_CURRENT_BINARY_DIR}/bin/prof.fdata
+  )
+  add_custom_target(clang-bolt
+DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/bin/clang.bolt
+)
+endif()
+
 if (LLVM_ADD_NATIVE_VISUALIZERS_TO_SOLUTION)
   add_subdirectory(utils/ClangVisualizers)
 endif()


Index: clang/cmake/caches/BOLT.cmake
===
--- /dev/null
+++ clang/cmake/caches/BOLT.cmake
@@ -0,0 +1,12 @@
+set(CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(CLANG_BOLT_INSTRUMENT ON CACHE BOOL "")
+set(CMAKE_EXE_LINKER_FLAGS "-Wl,--emit-relocs,-znow" CACHE STRING "")
+
+set(LLVM_ENABLE_PROJECTS "bolt;clang;lld" CACHE STRING "")
+set(LLVM_TARGETS_TO_BUILD Native CACHE STRING "")
+
+# Disable function splitting enabled by default in GCC8+
+if("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU")
+  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-reorder-blocks-and-partition")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-reorder-blocks-and-partition")
+endif()
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ 

[PATCH] D132713: [clang-tidy] Skip union-like classes in use-equals-default

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp:220
 void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) {
-  // Skip unions since constructors with empty bodies behave differently
-  // in comparison with structs/classes.
+  // Skip union-like classes since constructors with empty bodies behave
+  // differently in comparison with structs/classes.

gribozavr2 wrote:
> 
I've come across "union-like" classes here: 
https://en.cppreference.com/w/cpp/language/union
but yeah, will rephrase the comment.



Comment at: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp:225
+  anyOf(isUnion(),
+has(fieldDecl(isImplicit(), hasType(cxxRecordDecl(isUnion()));
 

gribozavr2 wrote:
> Why is "isImplicit" needed?
the intention for this patch was to skip classes containing anonymous unions
https://en.cppreference.com/w/cpp/language/union
In this case Clang's AST looks like this:
CXXRecordDecl - /* union */ FieldDecl (implicit) - IndirectFieldDecl - 
IndirectFieldDecl ...
For regular unions I haven't encountered issues yet (on a few large codebases)
(but yeah, might have to revisit it in the future).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132713

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

[PATCH] D132971: [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin module

[Ian Anderson via Phabricator via cfe-commits]

iana added a comment.

In D132971#3759538 , @Bigcheese wrote:

> I'm fine with this change, but do we actually have a backwards compatibility 
> policy anywhere in Clang? Would be good to know what range of SDKs a compiler 
> release is expected to support.

I don't know about backwards compatibility, but I found the Darwin module added 
the attribute in 2017 macOS 10.13/iOS 11.0. As far as I can tell from the 
original change, adding the attribute didn't actually fix anything, it was just 
used to detect cycles between libc++ and Darwin, so I think removing the hard 
code wouldn't even break older OSes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132971

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

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

[Jan Svoboda via Phabricator via cfe-commits]

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

LGTM. Up to you if you act on the last comment. Thanks for seeing this through!




Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:295
+if (MDC)
+  MDC->applyDiscoveredDependencies(CI);
+LastCC1Arguments = CI.getCC1CommandLine();

I'm not a fan of storing `MDC` as a member, but I don't see any clearly better 
alternatives. One option would be to pass `CompilerInvocation` out-parameter to 
`ModuleDepCollector`'s constructor and relying on the collector to apply 
discovered dependencies, but that's not super obvious.


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

https://reviews.llvm.org/D132405

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

[PATCH] D131268: [HLSL] Generate buffer subscript operators

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 456774.
beanz added a comment.

Updating based on @aaron.ballman's feedback.

- Change reinterpret_cast -> static_cast
- Aaron likes `auto` more than me... but all in good places :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131268

Files:
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/HLSL/RWBuffer-AST.hlsl
  clang/test/CodeGenHLSL/buffer-array-operator.hlsl

Index: clang/test/CodeGenHLSL/buffer-array-operator.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/buffer-array-operator.hlsl
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+const RWBuffer In;
+RWBuffer Out;
+
+void fn(int Idx) {
+  Out[Idx] = In[Idx];
+}
+
+// This test is intended to verify reasonable code generation of the subscript
+// operator. In this test case we should be generating both the const and
+// non-const operators so we verify both cases.
+
+// Non-const comes first.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QBAAAMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
+
+// Const comes next, and returns the pointer instead of the value.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
Index: clang/test/AST/HLSL/RWBuffer-AST.hlsl
===
--- clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -39,11 +39,30 @@
 
 // CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
 // CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>  implicit h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>  implicit h 'element_type *'
+
+// CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <>  operator[] 'element_type  (unsigned int) const'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <>  Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'const RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
+// CHECK-NEXT: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <>  operator[] 'element_type &(unsigned int)'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <>  Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
 // CHECK: ClassTemplateSpecializationDecl 0x{{[0-9A-Fa-f]+}} <>  class RWBuffer definition
 
 // CHECK: TemplateArgument type 'float'
 // CHECK-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
 // CHECK-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
 // CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>   implicit referenced h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>   implicit referenced h 'float *'
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2174,7 +2174,7 @@
 return QualType();
   }
 
-  if (getLangOpts().HLSL) {
+  if (getLangOpts().HLSL && Loc.isValid()) {
 Diag(Loc, diag::err_hlsl_pointers_unsupported) << 0;
 return QualType();
   }
@@ -2244,7 +2244,7 @@
 return QualType();
   }
 
-  if (getLangOpts().HLSL) {
+  if (getLangOpts().HLSL && Loc.isValid()) {
 Diag(Loc, diag::err_hlsl_pointers_unsupported) << 1;
 return QualType();
   }
@@ -3008,7 +3008,7 @@
 return QualType();
   }
 
-  if (getLangOpts().HLSL) {
+  

[PATCH] D132971: [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin module

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese accepted this revision.
Bigcheese added a comment.

I'm fine with this change, but do we actually have a backwards compatibility 
policy anywhere in Clang? Would be good to know what range of SDKs a compiler 
release is expected to support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132971

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

[PATCH] D132971: [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin module

[Juergen Ributzka via Phabricator via cfe-commits]

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

Seems straightforward. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132971

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

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

xbolva00 added a comment.

This commit caused following regression

https://github.com/llvm/llvm-project/issues/57449#issuecomment-1232102039


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120305

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

[PATCH] D132971: [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin module

[Ian Anderson via Phabricator via cfe-commits]

iana created this revision.
iana added reviewers: bruno, ributzka, vsapsai, Bigcheese.
Herald added a project: All.
iana requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The Darwin module has specified [no_undeclared_includes] for at least five 
years now, there's no need to hard code it in the compiler.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132971

Files:
  clang/lib/Lex/ModuleMap.cpp


Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -2026,8 +2026,7 @@
 ActiveModule->IsSystem = true;
   if (Attrs.IsExternC)
 ActiveModule->IsExternC = true;
-  if (Attrs.NoUndeclaredIncludes ||
-  (!ActiveModule->Parent && ModuleName == "Darwin"))
+  if (Attrs.NoUndeclaredIncludes)
 ActiveModule->NoUndeclaredIncludes = true;
   ActiveModule->Directory = Directory;
 


Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -2026,8 +2026,7 @@
 ActiveModule->IsSystem = true;
   if (Attrs.IsExternC)
 ActiveModule->IsExternC = true;
-  if (Attrs.NoUndeclaredIncludes ||
-  (!ActiveModule->Parent && ModuleName == "Darwin"))
+  if (Attrs.NoUndeclaredIncludes)
 ActiveModule->NoUndeclaredIncludes = true;
   ActiveModule->Directory = Directory;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D132952#3759398 , @efriedma wrote:

> In D132952#3759226 , @aaron.ballman 
> wrote:
>
>>> We could theoretically mess with the AST representation somehow, but I 
>>> don't see any compelling reason to.  We probably just want to emit the 
>>> warn_vla_used diagnostic somewhere else.
>>
>> Yeah, re-reading the C standard, I think I got VM types slightly wrong. 
>> Consider:
>>
>>   void func(int n, int array[n]);
>>
>> `array` is a VLA (C2x 6.7.6.2p4) before pointer adjustment (C2x 6.7.6.3p6), 
>> which makes `func` a VM type (C2x 6.7.6p3), not `array`.
>
> The type of parameter array after promotion (which is the type that matters 
> for almost any purpose), is `int*`.  That isn't variably modified.  And the 
> function type is just `void(int,int*)`; there's no way we can possibly treat 
> that as variably modified.

On the one hand, yes, that makes sense to me. On the other hand, 6.7.6p3 says:

A full declarator is a declarator that is not part of another declarator. If, 
in the nested sequence of
declarators in a full declarator, there is a declarator specifying a variable 
length array type, the type
specified by the full declarator is said to be variably modified. Furthermore, 
any type derived by
declarator type derivation from a variably modified type is itself variably 
modified.

`array` is not a full declarator in my example, `func` is, but the standard 
waves its hands around a lot regarding when the adjustments to function 
parameters happen. Notionally, I think they only impact the definition of the 
function body. So I think the function declaration type is `void(int, int[])` 
and I think the decayed function type on function call is `void (*)(int, int 
*)`, but I'm not 100% sure. But that's why I was thinking `func` is a VM type.

> If you wrote, say `void func(int n, int array[n][n])`, then array is variably 
> modified.  "func" is still not variably modified, I think?  "array" is not 
> part of the "nested sequence of declarators", despite the fact that the 
> parameter list contains a variably modified type.  At least, that's clang's 
> current interpretation of the rules.

Once upon a time I had convinced myself that Clang got our interpretation wrong 
here, and I was hoping I'd rediscover exactly why I thought that as I worked my 
way through the C DRs and C feature statuses by adding actual test coverage. A 
lot of Clang's C functionality is less rigorously tested, especially the older 
stuff, so it might help to start pulling together more rigorous testing in this 
area. That would also help us to identify situations where we're not quite 
certain what the standard says, and I can go back to WG14 with a list of 
questions.

At the very least, I don't read the C grammar as supporting that interpretation 
of the rules. `declarator` reaches `direct-declarator` which reaches 
`function-declarator` which reaches `parameter-type-list` which reaches 
`parameter-list` then `parameter-declaration`, which finally gets us back to 
`declarator` for us to start parsing `array` which ends up being a VLA. So 
`func` is the full declarator in that case and thus is VM, at least as I read 
the spec.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132952

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

[PATCH] D132932: [Clang][Comments] Parse `` in doc comments correctly

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/tools/libclang/CXString.cpp:85
+  if (String.empty())
+return createEmpty();
+

egorzhdan wrote:
> gribozavr2 wrote:
> > Please split this change into a separate patch and add a unit test.
> > 
> Do you know a good way to add a unit-test for this? This function only gets 
> called from within libclang, it isn't exposed to clients of libclang, 
> including libclangTest. So I think a unit test would probably need to invoke 
> the same CXComment API that `c-index-test` invokes while running 
> `comment-to-html-xml-conversion.cpp`. Would that be worth it?
It is unfortunate that the headers for CXString are not visible to 
llvm-project/clang/unittests/libclang/LibclangTest.cpp.

It wouldn't make sense to use the CXComment API because that's exactly what 
comment-to-html-xml-conversion.cpp does. A benefit of the unit test would be 
clear, direct testing of CXString.

Anyhow, please split the CXString change to a separate patch (without tests, 
unfortunately).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132932

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

[PATCH] D131153: AArch64: disable asynchronous unwind by default for MachO.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Driver/ToolChain.h:501
+  /// IsAsyncUnwindTablesDefault - Does this tool chain use
+  /// -fasync-unwind-tables by default.
+  virtual bool

smeenai wrote:
> I believe the option is spelled `-fasynchronous-unwind-tables`.
Changing `IsUnwindTablesDefault` to `unwindTablesDefaultLevel` may be cleaner. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131153

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

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

I looked at the new `Concept auto` changes, they seem fine.

Thanks a LOT @ychen for working on this, it's been a very interesting patch to 
me :)
I'll let @mizvekov accept since he is much more experienced than me in those 
areas.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128750

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

[PATCH] D132911: [clang-format] Fix annotating when deleting array of pointers

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

HazardyKnusperkeks added a comment.

Please mark comments as done, when the discussion has ended.




Comment at: clang/lib/Format/TokenAnnotator.cpp:2376
+const FormatToken *Prev = PrevToken;
+if (Prev->is(tok::r_square) && (Prev = Prev->getPreviousNonComment()) &&
+Prev->is(tok::l_square) && (Prev = Prev->getPreviousNonComment()) &&

I don't know if this is nice. I think I would prefer 
`PrevToken->getPreviousNonComment()` and subsequently 
`PrevToken->getPreviousNonComment()->getPreviousNonComment()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132911

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

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

In D132952#3759226 , @aaron.ballman 
wrote:

>> We could theoretically mess with the AST representation somehow, but I don't 
>> see any compelling reason to.  We probably just want to emit the 
>> warn_vla_used diagnostic somewhere else.
>
> Yeah, re-reading the C standard, I think I got VM types slightly wrong. 
> Consider:
>
>   void func(int n, int array[n]);
>
> `array` is a VLA (C2x 6.7.6.2p4) before pointer adjustment (C2x 6.7.6.3p6), 
> which makes `func` a VM type (C2x 6.7.6p3), not `array`.

The type of parameter array after promotion (which is the type that matters for 
almost any purpose), is `int*`.  That isn't variably modified.  And the 
function type is just `void(int,int*)`; there's no way we can possibly treat 
that as variably modified.

If you wrote, say `void func(int n, int array[n][n])`, then array is variably 
modified.  "func" is still not variably modified, I think?  "array" is not part 
of the "nested sequence of declarators", despite the fact that the parameter 
list contains a variably modified type.  At least, that's clang's current 
interpretation of the rules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132952

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

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

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

HazardyKnusperkeks added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:785
+  - Consecutive
+  - AcrossEmptyLines
+  - AcrossComments

yusuke-kadowaki wrote:
> MyDeveloperDay wrote:
> > may be AcrossEmptyLines should be a number (to mean the number of empty 
> > lines)
> We need to introduce a new struct to do that since AlignConsecutiveStyle is 
> shared with some options and not possible to be changed. Plus I think more 
> than two empty lines are formatted to one empty line anyway.
There `MaxEmptyLinesToKeep` which would allow to set it higher.

If we want to change the `bool` to an `unsigned`, then that should be a 
different change.



Comment at: clang/docs/ClangFormatStyleOptions.rst:794
+
+AlignConsecutiveMacros: AcrossEmptyLines
+

yusuke-kadowaki wrote:
> MyDeveloperDay wrote:
> > Should this say `AlignedConsecutuveCommets`
> No. This part is a documentation about AlignConsecutiveStyle type, which is 
> appended automatically after all the options that take AlignConsecutiveStyle 
> type.
Which we could change to reflect that it's used for multiple options.



Comment at: clang/docs/ClangFormatStyleOptions.rst:797
+AlignConsecutiveMacros:
+  Enabled: true
+  AcrossEmptyLines: true

yusuke-kadowaki wrote:
> MyDeveloperDay wrote:
> > HazardyKnusperkeks wrote:
> > > MyDeveloperDay wrote:
> > > > why do we need Enabled?
> > > > 
> > > > isn't it just
> > > > 
> > > > ```
> > > > false:
> > > > AcrossEmptyLines: false
> > > > AcrossComments: false
> > > > 
> > > > true:
> > > > AcrossEmptyLines: true
> > > > AcrossComments: true
> > > > ```
> > > The struct is a bit older. And we need `Enabled`, one might wish to align 
> > > only really consecutive comments, as the option right now does. (Same for 
> > > macros, assignments, etc..)
> > I'm still not sure I understand, plus are those use cases you describe 
> > tested, I couldn't see that. 
> > 
> > If feels like what you are saying is that AlignConsecutiveStyle is used 
> > elsewhere and it has options that don't pertain to aligning comments? 
> > 
> > I couldn't tell if feel like this documentation is describing something 
> > other than AligningTrailingComments, if I'm confused users could be too? 
> As for the Enabled option,
> Enabled: true
> AcrossEmptyLines: false
> 
> is supposed to work in the same way as the old style AlignTrailingComments. 
> So the tests of AlignTrailingComments are the test cases.
> 
> For the documentation, I also thought it was a bit confusing when I first saw 
> it because the description of the option and the style is kind of connected. 
> But as I also mentioned above, this part is automatically append and it 
> affects all the other options that have AlignConsecutiveStyle if we change. 
> So I think it should be another patch even if we are to modify it.
> I'm still not sure I understand, plus are those use cases you describe 
> tested, I couldn't see that. 
> 
> If feels like what you are saying is that AlignConsecutiveStyle is used 
> elsewhere and it has options that don't pertain to aligning comments? 
> 
> I couldn't tell if feel like this documentation is describing something other 
> than AligningTrailingComments, if I'm confused users could be too? 

It was added in D93986 as an enum, and in D119599 made into a struct which then 
had 2 options only valid for assignments, not the macros or declarations. Both 
accepted by you.

So I see no problem, but think it is the right thing, to port aligning comments 
to the struct, even if the flag `AccrossComments` is a noop in this case. When 
this lands I actually plan to add a flag only used by comments.




Comment at: clang/unittests/Format/FormatTestComments.cpp:2863
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"

Interesting would be a comment which is split, do we continue to align, or not?



Comment at: clang/unittests/Format/FormatTestComments.cpp:2958
+   "   long b;",
+   Style));   
+}

Trailing whitespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D131268: [HLSL] Generate buffer subscript operators

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:243
+AST, MethodDecl->getDeclContext(), SourceLocation(), SourceLocation(),
+, AST.UnsignedIntTy,
+AST.getTrivialTypeSourceInfo(AST.UnsignedIntTy, SourceLocation()),

aaron.ballman wrote:
> Should this be using a size type instead?
As currently implemented it is an unsigned int. 

I should include the doc link in the description:
https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/sm5-object-rwbuffer-operatorindex

Eventually I think we'll need to introduce size types to HLSL, but we currently 
don't really use them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131268

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

[PATCH] D132906: [Clang] Fix lambda CheckForDefaultedFunction(...) so that it checks the CXXMethodDecl is a special member function before attempting to call DefineDefaultedFunction(...)

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a minor test formatting nit. Please be sure to also write a 
release note for the fix.




Comment at: clang/test/SemaCXX/constant-expression-cxx2a.cpp:1478-1482
+class B{
+  virtual int constexpr f() = 0;
+};
+
+class D : B{




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

https://reviews.llvm.org/D132906

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

[PATCH] D127695: WIP: clang: Implement Template Specialization Resugaring

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: libcxx/utils/ci/buildkite-pipeline.yml:377
+CFLAGS: "-fcrash-diagnostics-dir=crash_diagnostics_dir"
+CXXFLAGS: "-fcrash-diagnostics-dir=crash_diagnostics_dir"
 LLVM_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer-${LLVM_HEAD_VERSION}"

Mordante wrote:
> I actually think this change would be nice for libc++ in general. I've has 
> several assertion failures in the bootstrapping CI. Especially with the 
> symbolizer available the crash reports become a lot better.
> 
> I'm not convinced the way you used the `CXXFLAGS` in the CMakeLists.txt is 
> the best way, maybe @ldionne has a better suggestion.
Ah, this is just an idea at this point, this is not even working for crashes in 
lit-tests, the `LIBCXX_TEST_COMPILER_FLAGS` setting below did not seem to work.

I would really appreciate suggestions, but meanwhile I'll try to get something 
minimally working here and spin a separate DR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127695

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

[PATCH] D132874: [clang] Don't emit debug vtable information for consteval functions

[Luke Nihlen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc9aba6007451: [clang] Dont emit debug vtable 
information for consteval functions (authored by luken-google).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132874

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/cxx20-consteval-crash.cpp


Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-unknown-linux-gnu 
-std=c++20 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-obj -debug-info-kind=constructor -std=c++20 %s -o -
 
 namespace PR50787 {
 // This code would previously cause a crash.
@@ -71,3 +72,23 @@
   return function(Item{'a'}, Item{'a'});
 }
 } // namespace Issue58871
+
+namespace Issue55065 {
+struct Base {
+  consteval virtual int Get() const = 0;
+};
+
+struct Derived : Base {
+  consteval int Get() const override {
+return 42;
+  }
+};
+
+int foo() {
+  constexpr Derived a;
+
+  auto val = a.Get();
+  return val;
+}
+} // namespace Issue55065
+
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -26,6 +26,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/VTableBuilder.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
@@ -1758,7 +1759,7 @@
   llvm::DISubprogram::DISPFlags SPFlags = llvm::DISubprogram::SPFlagZero;
   int ThisAdjustment = 0;
 
-  if (Method->isVirtual()) {
+  if (VTableContextBase::hasVtableSlot(Method)) {
 if (Method->isPure())
   SPFlags |= llvm::DISubprogram::SPFlagPureVirtual;
 else
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -198,10 +198,11 @@
   and `DR1734 
`_.
 - Class member variables are now in scope when parsing a ``requires`` clause. 
Fixes
   `GH55216 `_.
-
 - Correctly set expression evaluation context as 'immediate function context' 
in
   consteval functions.
   This fixes `GH51182 `
+- Fixes an assert crash caused by looking up missing vtable information on 
``consteval``
+  virtual functions. Fixes `GH55065 
`_.
 
 
 C++2b Feature Support


Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-unknown-linux-gnu -std=c++20 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-obj -debug-info-kind=constructor -std=c++20 %s -o -
 
 namespace PR50787 {
 // This code would previously cause a crash.
@@ -71,3 +72,23 @@
   return function(Item{'a'}, Item{'a'});
 }
 } // namespace Issue58871
+
+namespace Issue55065 {
+struct Base {
+  consteval virtual int Get() const = 0;
+};
+
+struct Derived : Base {
+  consteval int Get() const override {
+return 42;
+  }
+};
+
+int foo() {
+  constexpr Derived a;
+
+  auto val = a.Get();
+  return val;
+}
+} // namespace Issue55065
+
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -26,6 +26,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/VTableBuilder.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
@@ -1758,7 +1759,7 @@
   llvm::DISubprogram::DISPFlags SPFlags = llvm::DISubprogram::SPFlagZero;
   int ThisAdjustment = 0;
 
-  if (Method->isVirtual()) {
+  if (VTableContextBase::hasVtableSlot(Method)) {
 if (Method->isPure())
   SPFlags |= llvm::DISubprogram::SPFlagPureVirtual;
 else
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -198,10 +198,11 @@
   and `DR1734 `_.
 - Class member variables are now in scope when parsing a ``requires`` clause. Fixes
   `GH55216 

[clang] c9aba60 - [clang] Don't emit debug vtable information for consteval functions

[Luke Nihlen via cfe-commits]

Author: Luke Nihlen
Date: 2022-08-30T19:10:15Z
New Revision: c9aba600745131fca4f7333d7c2e21556c2577cc

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

LOG: [clang] Don't emit debug vtable information for consteval functions

Fixes https://github.com/llvm/llvm-project/issues/55065

Reviewed By: shafik

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGenCXX/cxx20-consteval-crash.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 824ab42706334..f946018b361c3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -198,10 +198,11 @@ C++20 Feature Support
   and `DR1734 
`_.
 - Class member variables are now in scope when parsing a ``requires`` clause. 
Fixes
   `GH55216 `_.
-
 - Correctly set expression evaluation context as 'immediate function context' 
in
   consteval functions.
   This fixes `GH51182 `
+- Fixes an assert crash caused by looking up missing vtable information on 
``consteval``
+  virtual functions. Fixes `GH55065 
`_.
 
 
 C++2b Feature Support

diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 3d78a13d30aa8..4b7e290f6480c 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -26,6 +26,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/VTableBuilder.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
@@ -1758,7 +1759,7 @@ llvm::DISubprogram *CGDebugInfo::CreateCXXMemberFunction(
   llvm::DISubprogram::DISPFlags SPFlags = llvm::DISubprogram::SPFlagZero;
   int ThisAdjustment = 0;
 
-  if (Method->isVirtual()) {
+  if (VTableContextBase::hasVtableSlot(Method)) {
 if (Method->isPure())
   SPFlags |= llvm::DISubprogram::SPFlagPureVirtual;
 else

diff  --git a/clang/test/CodeGenCXX/cxx20-consteval-crash.cpp 
b/clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
index 8aa87187d6c2f..da1c78db9af56 100644
--- a/clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ b/clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-unknown-linux-gnu 
-std=c++20 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-obj -debug-info-kind=constructor -std=c++20 %s -o -
 
 namespace PR50787 {
 // This code would previously cause a crash.
@@ -71,3 +72,23 @@ int foo() {
   return function(Item{'a'}, Item{'a'});
 }
 } // namespace Issue58871
+
+namespace Issue55065 {
+struct Base {
+  consteval virtual int Get() const = 0;
+};
+
+struct Derived : Base {
+  consteval int Get() const override {
+return 42;
+  }
+};
+
+int foo() {
+  constexpr Derived a;
+
+  auto val = a.Get();
+  return val;
+}
+} // namespace Issue55065
+



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

[PATCH] D132056: [HLSL] Restrict to supported targets

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 456746.
beanz added a comment.

Adding the FIXME suggested by @aaron.ballman. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132056

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/hlsl-lang-targets.hlsl
  clang/test/Preprocessor/predefined-macros-hlsl.c


Index: clang/test/Preprocessor/predefined-macros-hlsl.c
===
--- clang/test/Preprocessor/predefined-macros-hlsl.c
+++ clang/test/Preprocessor/predefined-macros-hlsl.c
@@ -29,20 +29,20 @@
 // PIXEL: #define __SHADER_TARGET_STAGE 0
 // VERTEX: #define __SHADER_TARGET_STAGE 1
 
-// RUN: %clang_cc1 %s -E -dM -o - -x hlsl -std=hlsl2015 | FileCheck 
-match-full-lines %s --check-prefixes=STD2015
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library %s -E -dM -o - -x 
hlsl -std=hlsl2015 | FileCheck -match-full-lines %s --check-prefixes=STD2015
 // STD2015: #define __HLSL_VERSION 2015
 
-// RUN: %clang_cc1 %s -E -dM -o - -x hlsl -std=hlsl2016 | FileCheck 
-match-full-lines %s --check-prefixes=STD2016
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library %s -E -dM -o - -x 
hlsl -std=hlsl2016 | FileCheck -match-full-lines %s --check-prefixes=STD2016
 // STD2016: #define __HLSL_VERSION 2016
 
-// RUN: %clang_cc1 %s -E -dM -o - -x hlsl -std=hlsl2017 | FileCheck 
-match-full-lines %s --check-prefixes=STD2017
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library %s -E -dM -o - -x 
hlsl -std=hlsl2017 | FileCheck -match-full-lines %s --check-prefixes=STD2017
 // STD2017: #define __HLSL_VERSION 2017
 
-// RUN: %clang_cc1 %s -E -dM -o - -x hlsl -std=hlsl2018 | FileCheck 
-match-full-lines %s --check-prefixes=STD2018
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library %s -E -dM -o - -x 
hlsl -std=hlsl2018 | FileCheck -match-full-lines %s --check-prefixes=STD2018
 // STD2018: #define __HLSL_VERSION 2018
 
-// RUN: %clang_cc1 %s -E -dM -o - -x hlsl -std=hlsl2021 | FileCheck 
-match-full-lines %s --check-prefixes=STD2021
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library %s -E -dM -o - -x 
hlsl -std=hlsl2021 | FileCheck -match-full-lines %s --check-prefixes=STD2021
 // STD2021: #define __HLSL_VERSION 2021
 
-// RUN: %clang_cc1 %s -E -dM -o - -x hlsl -std=hlsl202x | FileCheck 
-match-full-lines %s --check-prefixes=STD202x
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library %s -E -dM -o - -x 
hlsl -std=hlsl202x | FileCheck -match-full-lines %s --check-prefixes=STD202x
 // STD202x: #define __HLSL_VERSION 2029
Index: clang/test/Driver/hlsl-lang-targets.hlsl
===
--- /dev/null
+++ clang/test/Driver/hlsl-lang-targets.hlsl
@@ -0,0 +1,14 @@
+// RUN: not %clang -target x86_64-unknown-unknown %s 2>&1 | FileCheck %s 
--check-prefix=X86
+// RUN: not %clang -target dxil-unknown-unknown %s 2>&1 | FileCheck %s 
--check-prefix=DXIL
+// RUN: not %clang -target x86_64-unknown-shadermodel %s 2>&1 | FileCheck %s 
--check-prefix=SM
+// RUN: not %clang -target spirv64-unknown-unknown %s 2>&1 | FileCheck %s 
--check-prefix=SPIRV
+
+
+// A completely unsupported target...
+// X86: error: HLSL code generation is unsupported for target 
'x86_64-unknown-unknown'
+
+// Poorly specified targets
+// DXIL: error: HLSL code generation is unsupported for target 
'dxil-unknown-unknown'
+// SM: error: HLSL code generation is unsupported for target 
'x86_64-unknown-shadermodel'
+
+// FIXME// SPIRV: error: HLSL code generation is unsupported for target 
'spirv64-unknown-unknown'
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4111,6 +4111,14 @@
   if (const Arg *A = Args.getLastArg(OPT_frandomize_layout_seed_EQ))
 Opts.RandstructSeed = A->getValue(0);
 
+  // Validate options for HLSL
+  if (Opts.HLSL) {
+bool SupportedTarget = T.getArch() == llvm::Triple::dxil &&
+   T.getOS() == llvm::Triple::ShaderModel;
+if (!SupportedTarget)
+  Diags.Report(diag::err_drv_hlsl_unsupported_target) << T.str();
+  }
+
   return Diags.getNumErrors() == NumErrorsBefore;
 }
 
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -668,6 +668,8 @@
   "invalid profile : %0">;
 def err_drv_dxc_missing_target_profile : Error<
   "target profile option (-T) is missing">;
+def err_drv_hlsl_unsupported_target : Error<
+  "HLSL code generation is unsupported for target '%0'">;
 
 def err_drv_invalid_range_dxil_validator_version : Error<
   "invalid validator version : %0\n"


Index: 

[PATCH] D132945: [clang] Skip re-building lambda expressions in parameters to consteval fns.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 456745.
usaxena95 added a comment.

This actually fixes two more issues.
Update release notes and add more documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132945

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -939,3 +939,77 @@
 static_assert(max(1,2)==2);
 static_assert(mid(1,2,3)==2);
 } // namespace GH51182
+
+// https://github.com/llvm/llvm-project/issues/56183
+namespace GH56183 {
+consteval auto Foo(auto c) { return c; }
+consteval auto Bar(auto f) { return f(); }
+void test() {
+  constexpr auto x = Foo(Bar([] { return 'a'; }));
+  static_assert(x == 'a');
+}
+}  // namespace GH56183
+
+// https://github.com/llvm/llvm-project/issues/51695
+namespace GH51695 {
+// Original 
+template 
+struct type_t {};
+
+template 
+struct list_t {};
+
+template 
+consteval auto pop_front(list_t) -> auto {
+  return list_t{};
+}
+
+template 
+consteval auto apply(list_t, F fn) -> auto {
+  return fn(type_t{}...);
+}
+
+void test1() {
+  constexpr auto x = apply(pop_front(list_t{}),
+[](type_t...) { return 42; });
+  static_assert(x == 42);
+}
+// Reduced 1 
+consteval bool zero() { return false; }
+
+template 
+consteval bool foo(bool, F f) {
+  return f();
+}
+
+void test2() {
+  constexpr auto x = foo(zero(), []() { return true; });
+  static_assert(x);
+}
+
+// Reduced 2 
+template 
+consteval auto bar(F f) { return f;}
+
+void test3() {
+  constexpr auto x = bar(bar(bar(bar([]() { return true; }();
+  static_assert(x);
+
+  int a = 1; // expected-note {{declared here}}
+  auto y = bar(bar(bar(bar([=]() { return a; }(); // expected-error-re {{call to consteval function 'GH51695::bar<(lambda at {{.*}})>' is not a constant expression}}
+  // expected-note@-1 {{read of non-const variable 'a' is not allowed in a constant expression}}
+}
+
+}  // namespace GH51695
+
+// https://github.com/llvm/llvm-project/issues/50455
+namespace GH50455 {
+void f() {
+  []() consteval { int i{}; }();
+  []() consteval { int i{}; ++i; }();
+}
+void g() {
+  (void)[](int i) consteval { return i; }(0);
+  (void)[](int i) consteval { return i; }(0);
+}
+}  // namespace GH50455
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17598,6 +17598,11 @@
   DRSet.erase(E);
   return E;
 }
+ExprResult TransformLambdaExpr(LambdaExpr *E) {
+  // Lambdas must be built already. They must not be re-built as it always
+  // creates new types.
+  return E;
+}
 bool AlwaysRebuild() { return false; }
 bool ReplacingOriginal() { return true; }
 bool AllowSkippingCXXConstructExpr() {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -191,8 +191,14 @@
 
 - Correctly set expression evaluation context as 'immediate function context' in
   consteval functions.
-  This fixes `GH51182 `
-
+  This fixes `GH51182 `_.
+
+- Skip re-building lambda expressions when they appear as parameters to an immediate invocation.
+  This fixes `GH56183 `_,
+  `GH51695 `_,
+  `GH50455 `_,
+  `GH54872 `_,
+  `GH54587 `_.
 
 C++2b Feature Support
 ^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131268: [HLSL] Generate buffer subscript operators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:108-113
+if (Template) {
+  if (auto TTD = dyn_cast(
+  Template->getTemplateParameters()->getParam(0)))
+Ty = Record->getASTContext().getPointerType(
+QualType(TTD->getTypeForDecl(), 0));
+}





Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:180
+if (Handle->getType().getCanonicalType() != AST.VoidPtrTy) {
+  Call = CXXReinterpretCastExpr::Create(
+  AST, Handle->getType(), VK_PRValue, CK_Dependent, Call, nullptr,

Can you use a `static_cast` to make this a wee bit safer, or is that not 
possible?



Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:231-232
+AST.getFunctionType(ReturnTy, {AST.UnsignedIntTy}, ExtInfo);
+auto TSInfo = AST.getTrivialTypeSourceInfo(MethodTy, SourceLocation());
+auto MethodDecl = CXXMethodDecl::Create(
+AST, Record, SourceLocation(),





Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:241
+IdentifierInfo  = AST.Idents.get("Idx", tok::TokenKind::identifier);
+auto IdxParam = ParmVarDecl::Create(
+AST, MethodDecl->getDeclContext(), SourceLocation(), SourceLocation(),





Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:243
+AST, MethodDecl->getDeclContext(), SourceLocation(), SourceLocation(),
+, AST.UnsignedIntTy,
+AST.getTrivialTypeSourceInfo(AST.UnsignedIntTy, SourceLocation()),

Should this be using a size type instead?



Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:252
+
+CXXThisExpr *This = new (AST)
+CXXThisExpr(SourceLocation(), MethodDecl->getThisType(), true);





Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:254
+CXXThisExpr(SourceLocation(), MethodDecl->getThisType(), true);
+Expr *HandleAccess = MemberExpr::CreateImplicit(
+AST, This, true, Handle, Handle->getType(), VK_LValue, OK_Ordinary);





Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:257
+
+Expr *IndexExpr = DeclRefExpr::Create(
+AST, NestedNameSpecifierLoc(), SourceLocation(), IdxParam, false,





Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:262
+
+Expr *Array =
+new (AST) ArraySubscriptExpr(HandleAccess, IndexExpr, ElemTy, 
VK_LValue,





Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:266
+
+Stmt *Return = ReturnStmt::Create(AST, SourceLocation(), Array, nullptr);
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131268

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

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D128750#3758750 , @mizvekov wrote:

> Just a first glance at the patch, will try to do a more comprehensive review 
> later.

Thanks!




Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1838
 
-QualType Replacement = Arg.getAsType();
 

mizvekov wrote:
> I think this change could be masking a bug.
> 
> The arguments to an instantiation should always be canonical.
> 
> We could implement one day instantiation with non-canonical args, but this 
> would not be the only change required to get this to work.
> 
> And regardless it should be done in separate patch with proper test cases :)
Agreed.

It turns out this is because I was using injected template arguments to 
instantiate a function declaration (to compare function parameter types between 
two function templates). The injected template type arguments seem to be not 
canonical. 

Now I realized that I don't need this instantiation. Comparing the function 
parameter canonical types directly should be fine since the template parameters 
are uniqued by their kind/index/level etc. which is comparable between two 
function templates.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128750

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

[PATCH] D127695: WIP: clang: Implement Template Specialization Resugaring

[Mark de Wever via Phabricator via cfe-commits]

Mordante added subscribers: ldionne, Mordante.
Mordante added inline comments.



Comment at: libcxx/utils/ci/buildkite-pipeline.yml:377
+CFLAGS: "-fcrash-diagnostics-dir=crash_diagnostics_dir"
+CXXFLAGS: "-fcrash-diagnostics-dir=crash_diagnostics_dir"
 LLVM_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer-${LLVM_HEAD_VERSION}"

I actually think this change would be nice for libc++ in general. I've has 
several assertion failures in the bootstrapping CI. Especially with the 
symbolizer available the crash reports become a lot better.

I'm not convinced the way you used the `CXXFLAGS` in the CMakeLists.txt is the 
best way, maybe @ldionne has a better suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127695

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

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D132952#3759109 , @efriedma wrote:

>> We have the VariableArrayType to represent a VLA type and I think we're 
>> using that type when we should be using a variably modified type
>
> The clang AST for function signatures encodes two types: the type as written, 
> and the type after promotion.  Semantic analysis always uses the type after 
> promotion, but the type as written is useful if you're trying to do source 
> rewriting, or something like that.

Agreed; we need the type as written for `-ast-print` support, as well as AST 
matchers, etc.

> We could theoretically mess with the AST representation somehow, but I don't 
> see any compelling reason to.  We probably just want to emit the 
> warn_vla_used diagnostic somewhere else.

Yeah, re-reading the C standard, I think I got VM types slightly wrong. 
Consider:

  void func(int n, int array[n]);

`array` is a VLA (C2x 6.7.6.2p4) before pointer adjustment (C2x 6.7.6.3p6), 
which makes `func` a VM type (C2x 6.7.6p3), not `array`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132952

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

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 456743.
ychen marked an inline comment as done.
ychen added a comment.

- For function templates, compare canonical types of funtion parameters 
directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaConcept.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -967,7 +967,7 @@
   
   
 https://wg21.link/p2113r0;>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,17 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T) > 2;
+template  concept E = D && alignof(T) > 1;
+
+struct A {};
+template  struct S {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,14 +20,85 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
+#if 0
+// FIXME: [temp.func.order]p6.2.1 is not implemented, matching GCC.
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+#endif
+
+template class U, typename... Z>
+void foo(T, U) = delete;
+template class U, typename... Z>
+void foo(T, U) = delete;
+template class U, typename... Z>
+void foo(T, U);
+
+void bar(S s) {
+  foo(0, s);
+}
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct B{ B()=delete; };
+template struct B { B()=delete; };
+template struct B {};
+
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some {};
+
+void f() {
+  B b;
+  Some c;
+}
+
+template struct Z; // expected-note {{template is declared here}}
+template struct Z; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct W1;
+template struct W1 {};
+
+template struct W2;
+template struct W2 {};
+
+template
+concept C1 = C && C;
+template
+concept D1 = D && C;
+
+template auto T> struct W3;
+template auto T> struct W3 {};
+
+template auto... T> struct W4;
+template auto... T> struct W4 {};
+
+struct W1<0> w1;
+struct W2<0> w2;
+struct W3<0> w3;
+struct W4<0> w4;
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
@@ -98,26 +98,31 @@
   static_assert(is_same_v()), void>); // expected-error {{call to 'bar' is ambiguous}}
 
   template
-  constexpr int goo(int a) requires AtLeast2 && true {
+  constexpr int goo(int a) requires AtLeast2 && true { // expected-note {{candidate function}}
 return 1;
   }
 
   template
-  constexpr int goo(const int b) requires AtLeast2 {
+  constexpr int goo(const int b) requires AtLeast2 { // expected-note {{candidate function}}
 return 2;
   }
 
-  // Only trailing requires clauses of redeclarations are compared for overload resolution.
+  // [temp.func.order] p5
+  //   Since, in a call context, such type deduction considers only parameters
+  //   for which there are explicit call arguments, some parameters are ignored
+  //   (namely, function parameter packs, parameters with default arguments, and
+  //   ellipsis parameters).
   template
-  constexpr 

[PATCH] D132944: [clang] cleanup -fstrict-flex-arrays implementation

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:910
   if (FD->getParent()->isUnion())
-return StrictFlexArraysLevel < 2;
+return true;
   RecordDecl::field_iterator FI(

This is a functional change (which is good, but the commit message needs to be 
adjusted). In current trunk
```
union X { int x[0]; };
int foo(X*x) { return x->x[2]; }
```
built with `-fstrict-flex-arrays=2 -fsanitize=array-bounds` would incorrectly 
report ubsan error, and this change fixes that.

I think this testcase can be added to clang/test/CodeGen/bounds-checking-fam.c. 
Probably this should also be nominated for backport to the 15.x branch (for the 
first point release I expect)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132944

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

[PATCH] D130867: [clang] adds builtin `std::invoke` and `std::invoke_r`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: libc++.
aaron.ballman added a comment.

Thanks for working on this; these sort of improvements to compile time overhead 
are very much appreciated!

Has this been tested against libc++ (our preccommit CI doesn't test that), and 
if so, do all the results come back clean from their test suite? Did you try 
out any other large scale C++ projects to make sure they don't break?




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8411
+  "can't invoke pointer-to-%select{data member|member function}0: expected "
+  "second argument to be a %select{reference|wrapee|pointer}1 to a class "
+  "compatible with %2, got %3">;

Something's off here, what's a "wrapee"? ("to be a wrapee to a class 
compatible" doesn't seem grammatically correct.)



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8413-8414
+  "compatible with %2, got %3">;
+def err_invoke_pointer_to_member_drops_qualifiers : Error<
+  "can't invoke pointer-to-member function: '%0' drops '%1' qualifier%s2">;
+def err_invoke_pointer_to_member_ref_qualifiers : Error<

Can we reuse `err_pointer_to_member_call_drops_quals` instead of making a new 
diagnostic?



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8415
+  "can't invoke pointer-to-member function: '%0' drops '%1' qualifier%s2">;
+def err_invoke_pointer_to_member_ref_qualifiers : Error<
+  "can't invoke pointer-to-member function: '%0' can only be called on an "

Same question here, but about `err_pointer_to_member_oper_value_classify` 
instead.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8418-8430
+def err_invoke_wrong_number_of_args : Error<
+  "can't invoke %select{function|block|pointer-to-member function}0: expected "
+  "%1 %select{argument|arguments}2, got %3">;
+def err_invoke_function_object : Error<
+  "can't invoke %0 function object: %select{no|%2}1 suitable "
+  "overload%s2 found%select{|, which makes choosing ambiguous}1">;
+def err_invoke_function_object_deleted : Error<

Can we modify existing diagnostics for these as well?

It seems like in most cases, it's a matter of adding a select to use `invoke` 
terminology in all of these cases, which is why I'm asking.



Comment at: clang/include/clang/Sema/Sema.h:4099
  UnaryOperatorKind Opc,
- const UnresolvedSetImpl ,
- Expr *input, bool RequiresADL = true);
+ const UnresolvedSetImpl , Expr *input,
+ bool RequiresADL = true,

Since we're already touching the line anyway...



Comment at: clang/include/clang/Sema/Sema.h:4101
+ bool RequiresADL = true,
+ bool IsStdInvoke = false);
 

Hmmm, I wonder if there's a way we can make this change without having to force 
every caller to think about `std::invoke` when they look at the signature for 
these calls? For example (and maybe this is a terrible idea, I'm thinking out 
loud), could we use an RAII object that specifies we're in a "building a call 
to invoke" context that is then checked within these functions (and we leave 
that RAII object before evaluating arguments to the invoke)?

I'm not strongly opposed to the current approach, but I'm worried that it adds 
complexity for an infrequent construct. I think we should try to keep the 
situations where we need information about a specific API as self-contained as 
possible because WG21 has shown more and more interest in the library being 
implemented in the compiler (so there's a scaling concern as well).



Comment at: clang/lib/Sema/SemaExpr.cpp:267
   auto *Ctor = dyn_cast(FD);
-  if (Ctor && Ctor->isInheritingConstructor())
-Diag(Loc, diag::err_deleted_inherited_ctor_use)
-<< Ctor->getParent()
-<< Ctor->getInheritedConstructor().getConstructor()->getParent();
-  else
-Diag(Loc, diag::err_deleted_function_use);
-  NoteDeletedFunction(FD);
+  if (!IsStdInvoke) {
+if (Ctor && Ctor->isInheritingConstructor())

Why do we want to suppress the diagnostic here?



Comment at: clang/lib/Sema/SemaExpr.cpp:14543
   if (Result.isNull()) {
-S.Diag(OpLoc, diag::err_typecheck_indirection_requires_pointer)
-  << OpTy << Op->getSourceRange();
+if (!IsStdInvoke)
+  S.Diag(OpLoc, diag::err_typecheck_indirection_requires_pointer)

This also surprises me.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5713-5715
+  if (B.isInvalid()) {
+return ExprError();
+  }

Down with braces! Up with danger!



Comment at: 

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

The reason struct returns require register shuffling is that AArch64 passes the 
sret pointer in x8 (i.e. RAX), but the x64 calling convention expects in in RCX 
(i.e. x0).

Have you tried to see if the Microsoft-generated thunk actually works?  I found 
at least one bug in MSVC thunk generation and reported it to Microsoft.  
(Microsoft didn't acknowledge the report, but that's a different story...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125418

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

[PATCH] D132945: [clang] Skip re-building lambda expressions in parameters to consteval fns.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 456739.
usaxena95 added a comment.

Add some test with expected errors as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132945

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -939,3 +939,77 @@
 static_assert(max(1,2)==2);
 static_assert(mid(1,2,3)==2);
 } // namespace GH51182
+
+// https://github.com/llvm/llvm-project/issues/56183
+namespace GH56183 {
+consteval auto Foo(auto c) { return c; }
+consteval auto Bar(auto f) { return f(); }
+void test() {
+  constexpr auto x = Foo(Bar([] { return 'a'; }));
+  static_assert(x == 'a');
+}
+}  // namespace GH56183
+
+// https://github.com/llvm/llvm-project/issues/51695
+namespace GH51695 {
+// Original 
+template 
+struct type_t {};
+
+template 
+struct list_t {};
+
+template 
+consteval auto pop_front(list_t) -> auto {
+  return list_t{};
+}
+
+template 
+consteval auto apply(list_t, F fn) -> auto {
+  return fn(type_t{}...);
+}
+
+void test1() {
+  constexpr auto x = apply(pop_front(list_t{}),
+[](type_t...) { return 42; });
+  static_assert(x == 42);
+}
+// Reduced 1 
+consteval bool zero() { return false; }
+
+template 
+consteval bool foo(bool, F f) {
+  return f();
+}
+
+void test2() {
+  constexpr auto x = foo(zero(), []() { return true; });
+  static_assert(x);
+}
+
+// Reduced 2 
+template 
+consteval auto bar(F f) { return f;}
+
+void test3() {
+  constexpr auto x = bar(bar(bar(bar([]() { return true; }();
+  static_assert(x);
+
+  int a = 1; // expected-note {{declared here}}
+  auto y = bar(bar(bar(bar([=]() { return a; }(); // expected-error-re {{call to consteval function 'GH51695::bar<(lambda at {{.*}})>' is not a constant expression}}
+  // expected-note@-1 {{read of non-const variable 'a' is not allowed in a constant expression}}
+}
+
+}  // namespace GH51695
+
+// https://github.com/llvm/llvm-project/issues/50455
+namespace GH50455 {
+void f() {
+  []() consteval { int i{}; }();
+  []() consteval { int i{}; ++i; }();
+}
+void g() {
+  (void)[](int i) consteval { return i; }(0);
+  (void)[](int i) consteval { return i; }(0);
+}
+}  // namespace GH50455
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17598,6 +17598,7 @@
   DRSet.erase(E);
   return E;
 }
+ExprResult TransformLambdaExpr(LambdaExpr *E) { return E; }
 bool AlwaysRebuild() { return false; }
 bool ReplacingOriginal() { return true; }
 bool AllowSkippingCXXConstructExpr() {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -191,8 +191,12 @@
 
 - Correctly set expression evaluation context as 'immediate function context' in
   consteval functions.
-  This fixes `GH51182 `
+  This fixes `GH51182 `_.
 
+- Skip re-building lambda expressions when they appear as parameters to an immediate invocation.
+  This fixes `GH56183 `_,
+  `GH51695 `_,
+  `GH50455 `_.
 
 C++2b Feature Support
 ^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132654: [clang-tidy] Fix false positive on `ArrayInitIndexExpr` inside `ProBoundsConstantArrayIndexCheck`

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcd40245f549b: [clang-tidy] Fix false positive on 
ArrayInitIndexExpr inside… (authored by isuckatcs).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132654

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
@@ -75,13 +75,34 @@
   s[i] = 3; // OK, custom operator
 }
 
+namespace ArrayInitIndexExpr {
 struct A {
   // The compiler-generated copy constructor uses an ArraySubscriptExpr. Don't 
warn.
   int x[3];
 };
 
-void use_A() {
+void implicitCopyMoveCtor() {
   // Force the compiler to generate a copy constructor.
   A a;
   A a2(a);
+
+  // Force the compiler to generate a move constructor.
+  A a3 = (A&&) a;
+}
+
+void lambdaCapture() {
+  int arr[3];
+
+  // Capturing an array by value uses an ArraySubscriptExpr. Don't warn. 
+  [arr](){};
+}
+
+#if __cplusplus >= 201703L
+void structuredBindings() {
+  int arr[3];
+
+  // Creating structured bindings by value uses an ArraySubscriptExpr. Don't 
warn.
+  auto [a,b,c] = arr;
 }
+#endif
+} // namespace ArrayInitIndexExpr
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -61,6 +61,12 @@
   const auto *Matched = Result.Nodes.getNodeAs("expr");
   const auto *IndexExpr = Result.Nodes.getNodeAs("index");
 
+  // This expression can only appear inside ArrayInitLoopExpr, which
+  // is always implicitly generated. ArrayInitIndexExpr is not a
+  // constant, but we shouldn't report a warning for it.
+  if (isa(IndexExpr))
+return;
+
   if (IndexExpr->isValueDependent())
 return; // We check in the specialization.
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
@@ -75,13 +75,34 @@
   s[i] = 3; // OK, custom operator
 }
 
+namespace ArrayInitIndexExpr {
 struct A {
   // The compiler-generated copy constructor uses an ArraySubscriptExpr. Don't warn.
   int x[3];
 };
 
-void use_A() {
+void implicitCopyMoveCtor() {
   // Force the compiler to generate a copy constructor.
   A a;
   A a2(a);
+
+  // Force the compiler to generate a move constructor.
+  A a3 = (A&&) a;
+}
+
+void lambdaCapture() {
+  int arr[3];
+
+  // Capturing an array by value uses an ArraySubscriptExpr. Don't warn. 
+  [arr](){};
+}
+
+#if __cplusplus >= 201703L
+void structuredBindings() {
+  int arr[3];
+
+  // Creating structured bindings by value uses an ArraySubscriptExpr. Don't warn.
+  auto [a,b,c] = arr;
 }
+#endif
+} // namespace ArrayInitIndexExpr
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -61,6 +61,12 @@
   const auto *Matched = Result.Nodes.getNodeAs("expr");
   const auto *IndexExpr = Result.Nodes.getNodeAs("index");
 
+  // This expression can only appear inside ArrayInitLoopExpr, which
+  // is always implicitly generated. ArrayInitIndexExpr is not a
+  // constant, but we shouldn't report a warning for it.
+  if (isa(IndexExpr))
+return;
+
   if (IndexExpr->isValueDependent())
 return; // We check in the specialization.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] cd40245 - [clang-tidy] Fix false positive on ArrayInitIndexExpr inside ProBoundsConstantArrayIndexCheck

[via cfe-commits]

Author: isuckatcs
Date: 2022-08-30T20:27:38+02:00
New Revision: cd40245f549b8bd7a5b6571c2eb6a882ce59acc9

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

LOG: [clang-tidy] Fix false positive on ArrayInitIndexExpr inside 
ProBoundsConstantArrayIndexCheck

Sometimes in the AST we can have an ArraySubscriptExpr,
where the index is an ArrayInitIndexExpr.
ArrayInitIndexExpr is not a constant, so
ProBoundsConstantArrayIndexCheck reports a warning when
it sees such expression. This expression can only be
implicitly generated, and always appears inside an
ArrayInitLoopExpr, so we shouldn't report a warning.

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

Added: 


Modified: 

clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
index 8c3457d9c5580..c80be3df1ce62 100644
--- 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -61,6 +61,12 @@ void ProBoundsConstantArrayIndexCheck::check(
   const auto *Matched = Result.Nodes.getNodeAs("expr");
   const auto *IndexExpr = Result.Nodes.getNodeAs("index");
 
+  // This expression can only appear inside ArrayInitLoopExpr, which
+  // is always implicitly generated. ArrayInitIndexExpr is not a
+  // constant, but we shouldn't report a warning for it.
+  if (isa(IndexExpr))
+return;
+
   if (IndexExpr->isValueDependent())
 return; // We check in the specialization.
 

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
index 7d5390ddecfb9..91c7fc1e3db59 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
@@ -75,13 +75,34 @@ void customOperator() {
   s[i] = 3; // OK, custom operator
 }
 
+namespace ArrayInitIndexExpr {
 struct A {
   // The compiler-generated copy constructor uses an ArraySubscriptExpr. Don't 
warn.
   int x[3];
 };
 
-void use_A() {
+void implicitCopyMoveCtor() {
   // Force the compiler to generate a copy constructor.
   A a;
   A a2(a);
+
+  // Force the compiler to generate a move constructor.
+  A a3 = (A&&) a;
+}
+
+void lambdaCapture() {
+  int arr[3];
+
+  // Capturing an array by value uses an ArraySubscriptExpr. Don't warn. 
+  [arr](){};
+}
+
+#if __cplusplus >= 201703L
+void structuredBindings() {
+  int arr[3];
+
+  // Creating structured bindings by value uses an ArraySubscriptExpr. Don't 
warn.
+  auto [a,b,c] = arr;
 }
+#endif
+} // namespace ArrayInitIndexExpr



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

[PATCH] D132962: [clangd][ObjC] Improve completions for protocols + category names

[David Goldman via Phabricator via cfe-commits]

dgoldman created this revision.
dgoldman added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
dgoldman requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

- Render protocols as interfaces to differentiate them from classes since a 
protocol and class can have the same name

- Properly call `includeSymbolFromIndex` even with a cached speculative fuzzy 
find request

- Don't use the index to provide completions for category names, symbols there 
don't make sense


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132962

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3215,7 +3215,10 @@
  /*Opts=*/{}, "Foo.m");
 
   auto C = Results.Completions;
-  EXPECT_THAT(C, UnorderedElementsAre(named("Food"), named("Fooey")));
+  EXPECT_THAT(C,
+  UnorderedElementsAre(
+  AllOf(named("Food"), kind(CompletionItemKind::Interface)),
+  AllOf(named("Fooey"), kind(CompletionItemKind::Interface;
 }
 
 TEST(CompletionTest, CursorInSnippets) {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -94,10 +94,14 @@
   case SK::Struct:
 return CompletionItemKind::Struct;
   case SK::Class:
-  case SK::Protocol:
   case SK::Extension:
   case SK::Union:
 return CompletionItemKind::Class;
+  case SK::Protocol:
+// Use interface instead of class for differentiation of classes and
+// protocols with the same name (e.g. @interface NSObject vs. @protocol
+// NSObject).
+return CompletionItemKind::Interface;
   case SK::TypeAlias:
 // We use the same kind as the VSCode C++ extension.
 // FIXME: pick a better option when we have one.
@@ -712,13 +716,13 @@
   case CodeCompletionContext::CCC_Type:
   case CodeCompletionContext::CCC_ParenthesizedExpression:
   case CodeCompletionContext::CCC_ObjCInterfaceName:
-  case CodeCompletionContext::CCC_ObjCCategoryName:
   case CodeCompletionContext::CCC_Symbol:
   case CodeCompletionContext::CCC_SymbolOrNewName:
 return true;
   case CodeCompletionContext::CCC_OtherWithMacros:
   case CodeCompletionContext::CCC_DotMemberAccess:
   case CodeCompletionContext::CCC_ArrowMemberAccess:
+  case CodeCompletionContext::CCC_ObjCCategoryName:
   case CodeCompletionContext::CCC_ObjCPropertyAccess:
   case CodeCompletionContext::CCC_MacroName:
   case CodeCompletionContext::CCC_MacroNameUse:
@@ -1343,6 +1347,22 @@
   llvm_unreachable("invalid NestedNameSpecifier kind");
 }
 
+// Should we include a symbol from the index given the completion kind?
+// FIXME: Ideally we can filter in the fuzzy find request itself.
+bool includeSymbolFromIndex(CodeCompletionContext::Kind Kind,
+const Symbol ) {
+  // Objective-C protocols are only useful in ObjC protocol completions,
+  // in other places they're confusing, especially when they share the same
+  // identifier with a class.
+  if (Sym.SymInfo.Kind == index::SymbolKind::Protocol &&
+  Sym.SymInfo.Lang == index::SymbolLanguage::ObjC)
+return Kind == CodeCompletionContext::CCC_ObjCProtocolName;
+  else if (Kind == CodeCompletionContext::CCC_ObjCProtocolName)
+// Don't show anything else in ObjC protocol completions.
+return false;
+  return true;
+}
+
 std::future startAsyncFuzzyFind(const SymbolIndex ,
 const FuzzyFindRequest ) {
   return runAsync([, Req]() {
@@ -1675,14 +1695,6 @@
 return Output;
   }
 
-  bool includeSymbolFromIndex(const Symbol ) {
-if (CCContextKind == CodeCompletionContext::CCC_ObjCProtocolName) {
-  return Sym.SymInfo.Lang == index::SymbolLanguage::ObjC &&
-  Sym.SymInfo.Kind == index::SymbolKind::Protocol;
-}
-return true;
-  }
-
   SymbolSlab queryIndex() {
 trace::Span Tracer("Query index");
 SPAN_ATTACH(Tracer, "limit", int64_t(Opts.Limit));
@@ -1716,10 +1728,8 @@
 
 // Run the query against the index.
 SymbolSlab::Builder ResultsBuilder;
-if (Opts.Index->fuzzyFind(Req, [&](const Symbol ) {
-  if (includeSymbolFromIndex(Sym))
-ResultsBuilder.insert(Sym);
-}))
+if (Opts.Index->fuzzyFind(
+Req, [&](const Symbol ) { ResultsBuilder.insert(Sym); }))
   Incomplete = true;
 return std::move(ResultsBuilder).build();
   }
@@ -1783,6 +1793,8 @@
 for (const auto  : IndexResults) {
   if (UsedIndexResults.count())
 continue;

[PATCH] D132913: [HLSL] Preserve vec3 for HLSL.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3524
+  // Preserve vec3 for HLSL.
+  CmdArgs.push_back("-fpreserve-vec3-type");
 }

Preserving vec3 is required for HLSL correctness, this shouldn't be tied to the 
DXC driver mode. Instead, the option should be implied by the HLSL language 
mode. You should be able to add the following to the TableGen definition for 
the `preserve-vec3-type` flag:

```
ImpliedByAnyOf<[hlsl.KeyPath]>;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132913

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

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> We have the VariableArrayType to represent a VLA type and I think we're using 
> that type when we should be using a variably modified type

The clang AST for function signatures encodes two types: the type as written, 
and the type after promotion.  Semantic analysis always uses the type after 
promotion, but the type as written is useful if you're trying to do source 
rewriting, or something like that.

We could theoretically mess with the AST representation somehow, but I don't 
see any compelling reason to.  We probably just want to emit the warn_vla_used 
diagnostic somewhere else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132952

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

[PATCH] D131268: [HLSL] Generate buffer subscript operators

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 456731.
beanz added a comment.

Updating based on PR feedback and rebasing.

- Rebased on main today
- Made const subscript return type const &


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131268

Files:
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/HLSL/RWBuffer-AST.hlsl
  clang/test/CodeGenHLSL/buffer-array-operator.hlsl

Index: clang/test/CodeGenHLSL/buffer-array-operator.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/buffer-array-operator.hlsl
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+const RWBuffer In;
+RWBuffer Out;
+
+void fn(int Idx) {
+  Out[Idx] = In[Idx];
+}
+
+// This test is intended to verify reasonable code generation of the subscript
+// operator. In this test case we should be generating both the const and
+// non-const operators so we verify both cases.
+
+// Non-const comes first.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QBAAAMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
+
+// Const comes next, and returns the pointer instead of the value.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
Index: clang/test/AST/HLSL/RWBuffer-AST.hlsl
===
--- clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -39,11 +39,30 @@
 
 // CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
 // CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>  implicit h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>  implicit h 'element_type *'
+
+// CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <>  operator[] 'element_type  (unsigned int) const'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <>  Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'const RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
+// CHECK-NEXT: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <>  operator[] 'element_type &(unsigned int)'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <>  Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
 // CHECK: ClassTemplateSpecializationDecl 0x{{[0-9A-Fa-f]+}} <>  class RWBuffer definition
 
 // CHECK: TemplateArgument type 'float'
 // CHECK-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
 // CHECK-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
 // CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>   implicit referenced h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>   implicit referenced h 'float *'
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2174,7 +2174,7 @@
 return QualType();
   }
 
-  if (getLangOpts().HLSL) {
+  if (getLangOpts().HLSL && Loc.isValid()) {
 Diag(Loc, diag::err_hlsl_pointers_unsupported) << 0;
 return QualType();
   }
@@ -2244,7 +2244,7 @@
 return QualType();
   }
 
-  if (getLangOpts().HLSL) {
+  if (getLangOpts().HLSL && Loc.isValid()) {
 Diag(Loc, diag::err_hlsl_pointers_unsupported) << 1;
 return QualType();
   }
@@ -3008,7 +3008,7 @@
 return QualType();
   }
 
-  if (getLangOpts().HLSL) {
+  if (getLangOpts().HLSL && 

[PATCH] D132911: [clang-format] Fix annotating when deleting array of pointers

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:2381-2385
 if (PrevToken->Tok.isLiteral() ||
 PrevToken->isOneOf(tok::r_paren, tok::r_square, tok::kw_true,
tok::kw_false, tok::r_brace)) {
   return TT_BinaryOperator;
 }

jackhong12 wrote:
> The `*` token in `delete *x;` will be annotated as UnaryOperator 
> (dereferencing a pointer). 
> 
> In the case `delete[] *x;`, there is a `]` before `*`. So, it will be 
> annotated as BinaryOperator by this rule.
> 
> I think both `*` here should be UnaryOperator. Therefore, I add a new rule to 
> match the `delete[]` pattern.
 `it will be annotated as BinaryOperator by this rule.` of course my bad



Comment at: clang/unittests/Format/FormatTest.cpp:10541
+  verifyFormat("delete[] **ptr;", Style);
+  verifyFormat("delete[] *(ptr);", Style);
+

could you cover the other PointerAlignment case for completeness


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132911

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

[PATCH] D131153: AArch64: disable asynchronous unwind by default for MachO.

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

I just discovered this and was about to post on Discourse about it. Glad you're 
already on it :)

I don't think this is quite correct though? It'll turn off unwind tables for 
AArch64 entirely, whereas we want to keep sync unwind tables. If we want to 
minimize changes to the existing logic, maybe we could add 
`IsSyncUnwindTablesDefault` instead and have `SyncUnwindTables` default to that 
and `AsyncUnwindTables` default to `!` that?




Comment at: clang/include/clang/Driver/ToolChain.h:501
+  /// IsAsyncUnwindTablesDefault - Does this tool chain use
+  /// -fasync-unwind-tables by default.
+  virtual bool

I believe the option is spelled `-fasynchronous-unwind-tables`.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2898
+bool MachO::IsAsyncUnwindTablesDefault(const ArgList ) const {
+  // AArch64 has compact unwind format, making the size overhead from
+  // asynchronous unwind particularly bad, so disable by default.

This comment is a bit confusing. I thought Darwin supported compact unwind for 
all architectures. Is it that compact unwind can't handle async unwind on arm64 
but can on other architectures?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131153

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

[PATCH] D129488: [Sema] Delay evaluation of std::source_location::current() in default arguments

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D129488#3758798 , @ilya-biryukov 
wrote:

> I was also under impression that this should fall out of the C++ semantics, 
> but after playing around with it I am not so sure anymore.

I'm also starting to question that. :-)

>> Isn't it wrong to eagerly evaluate _other_ calls in default args, as well?
>> ISTM that source_location is simply _exposing_ the bug in where we evaluate 
>> these expressions, but that it's actually a more general problem?
>
> Yes, it will evaluate calls to other `consteval` functions and and I don't 
> think there is a way to notice this if there are no errors in the `consteval` 
> functions. `constexpr` and `consteval` functions should produce the same 
> value when called with the same argument IIUC.
> `source_location::current()` is special in that regard and it breaks that 
> assumption. E.g. the fact that `CXXDefaultArgExpr` references the same 
> expression from `ParmVarDecl`.
> We could delay evaluation of other `consteval` functions until the default 
> argument is actually used relatively cheaply for other functions, but 
> `source_location::current()` is the only one that requires a complete revamp 
> of how `CXXDefaultArgExpr` works (see D132941 
>  for an attempt, it's not pretty, unless 
> there is a simple way I missed).
>
> I am also torn on how to interpret the standard. It might be that evaluating 
> immediate function in default arguments in actually required.
> `[dcl.fct.default]p5` mentions we need to do full semantic checking of the 
> initializer for default arguments:
>
>   The default argument has the same semantic constraints as the initializer 
> in a declaration of a variable of the
>   parameter type, using the copy-initialization semantics (9.4). The names in 
> the default argument are bound,
>   and the semantic constraints are checked, at the point where the default 
> argument appears.
>
> Additionally, the standard requires all immediate invocations to be evaluated 
> (unless they are in immediate function context, which is not the case if 
> default arguments are not for consteval function).
> Hence, we should also evaluate the immediate invocations as part of the 
> semantic checking we do for initializer of the default argument.
>
> All major compilers certainly evaluate the `consteval` functions in the 
> default arguments even without calls to the function that has those 
> parameters:
> https://gcc.godbolt.org/z/qTzrf6Msx
>
> It might be an implementation quirk and a clarification would be nice. I am 
> leaning towards thinking this behavior is actually standard-compliant.
> This does not contradict the fact that default arguments must be evaluated in 
> the context of the caller.
> One can't see the difference for `consteval` functions as they must be 
> replaced by constants.
>
> Both GCC and MSVC seem to special-case `std::source_location::current()` from 
> what I can tell.

https://eel.is/c++draft/expr.call#7.sentence-3 is what causes the argument to 
be evaluated at the call expression rather than at the declaration while 
https://eel.is/c++draft/dcl.fct.default#5 makes it clear that the semantic 
constraints are checked at the declaration location. I couldn't find explicit 
wording which says this, but I believe that semantic checking in this case also 
involves determining whether the immediate function is a valid or not 
(https://eel.is/c++draft/expr.const#14), which should explain the behavior of 
your example in https://gcc.godbolt.org/z/qTzrf6Msx.

However, https://eel.is/c++draft/dcl.constexpr#5 says that the behavior has to 
be the same whether the constexpr function is runtime evaluated or constant 
evaluated, and a function declared `consteval` is a constexpr function 
(https://eel.is/c++draft/dcl.constexpr#2.sentence-1). That sure does make 
std::source_location::current() a rather special function -- if the call were 
to be evaluated at runtime (which it can't because it's an immediate function), 
it would give different results.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129488

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

[PATCH] D132654: [clang-tidy] Fix false positive on `ArrayInitIndexExpr` inside `ProBoundsConstantArrayIndexCheck`

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

isuckatcs updated this revision to Diff 456715.
isuckatcs marked an inline comment as done.
isuckatcs added a comment.

Removed the unnecessary extra RUN command.


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

https://reviews.llvm.org/D132654

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
@@ -75,13 +75,34 @@
   s[i] = 3; // OK, custom operator
 }
 
+namespace ArrayInitIndexExpr {
 struct A {
   // The compiler-generated copy constructor uses an ArraySubscriptExpr. Don't 
warn.
   int x[3];
 };
 
-void use_A() {
+void implicitCopyMoveCtor() {
   // Force the compiler to generate a copy constructor.
   A a;
   A a2(a);
+
+  // Force the compiler to generate a move constructor.
+  A a3 = (A&&) a;
+}
+
+void lambdaCapture() {
+  int arr[3];
+
+  // Capturing an array by value uses an ArraySubscriptExpr. Don't warn. 
+  [arr](){};
+}
+
+#if __cplusplus >= 201703L
+void structuredBindings() {
+  int arr[3];
+
+  // Creating structured bindings by value uses an ArraySubscriptExpr. Don't 
warn.
+  auto [a,b,c] = arr;
 }
+#endif
+} // namespace ArrayInitIndexExpr
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -61,6 +61,12 @@
   const auto *Matched = Result.Nodes.getNodeAs("expr");
   const auto *IndexExpr = Result.Nodes.getNodeAs("index");
 
+  // This expression can only appear inside ArrayInitLoopExpr, which
+  // is always implicitly generated. ArrayInitIndexExpr is not a
+  // constant, but we shouldn't report a warning for it.
+  if (isa(IndexExpr))
+return;
+
   if (IndexExpr->isValueDependent())
 return; // We check in the specialization.
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
@@ -75,13 +75,34 @@
   s[i] = 3; // OK, custom operator
 }
 
+namespace ArrayInitIndexExpr {
 struct A {
   // The compiler-generated copy constructor uses an ArraySubscriptExpr. Don't warn.
   int x[3];
 };
 
-void use_A() {
+void implicitCopyMoveCtor() {
   // Force the compiler to generate a copy constructor.
   A a;
   A a2(a);
+
+  // Force the compiler to generate a move constructor.
+  A a3 = (A&&) a;
+}
+
+void lambdaCapture() {
+  int arr[3];
+
+  // Capturing an array by value uses an ArraySubscriptExpr. Don't warn. 
+  [arr](){};
+}
+
+#if __cplusplus >= 201703L
+void structuredBindings() {
+  int arr[3];
+
+  // Creating structured bindings by value uses an ArraySubscriptExpr. Don't warn.
+  auto [a,b,c] = arr;
 }
+#endif
+} // namespace ArrayInitIndexExpr
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -61,6 +61,12 @@
   const auto *Matched = Result.Nodes.getNodeAs("expr");
   const auto *IndexExpr = Result.Nodes.getNodeAs("index");
 
+  // This expression can only appear inside ArrayInitLoopExpr, which
+  // is always implicitly generated. ArrayInitIndexExpr is not a
+  // constant, but we shouldn't report a warning for it.
+  if (isa(IndexExpr))
+return;
+
   if (IndexExpr->isValueDependent())
 return; // We check in the specialization.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] de8f372 - [Docs] Fixing incorrect document title

[Chris Bieneman via cfe-commits]

Author: Chris Bieneman
Date: 2022-08-30T12:19:05-05:00
New Revision: de8f372bfec7070a364fac57fb5f201fc2d8cb22

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

LOG: [Docs] Fixing incorrect document title

Doh! This clearly slipped my review. Thanks DuckDuckGo for showing me
the error of my ways :).

Added: 


Modified: 
clang/docs/HLSL/ResourceTypes.rst

Removed: 




diff  --git a/clang/docs/HLSL/ResourceTypes.rst 
b/clang/docs/HLSL/ResourceTypes.rst
index c537e824758b..aad7b3314f08 100644
--- a/clang/docs/HLSL/ResourceTypes.rst
+++ b/clang/docs/HLSL/ResourceTypes.rst
@@ -1,6 +1,6 @@
-
-HLSL Support
-
+===
+HLSL Resource Types
+===
 
 .. contents::
:local:



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

[PATCH] D132672: [Docs] [HLSL] Documenting HLSL Entry Functions

[Chris Bieneman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG739a747b2368: [Docs] [HLSL] Documenting HLSL Entry Functions 
(authored by beanz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132672

Files:
  clang/docs/HLSL/EntryFunctions.rst
  clang/docs/HLSL/HLSLDocs.rst


Index: clang/docs/HLSL/HLSLDocs.rst
===
--- clang/docs/HLSL/HLSLDocs.rst
+++ clang/docs/HLSL/HLSLDocs.rst
@@ -12,3 +12,4 @@
:maxdepth: 1
 
ResourceTypes
+   EntryFunctions
Index: clang/docs/HLSL/EntryFunctions.rst
===
--- /dev/null
+++ clang/docs/HLSL/EntryFunctions.rst
@@ -0,0 +1,65 @@
+
+HLSL Entry Functions
+
+
+.. contents::
+   :local:
+
+Usage
+=
+
+In HLSL, entry functions denote the starting point for shader execution. They
+must be known at compile time. For all non-library shaders, the compiler 
assumes
+the default entry function name ``main``, unless the DXC ``/E`` option is
+provided to specify an alternate entry point. For library shaders entry points
+are denoted using the ``[shader(...)]`` attribute.
+
+All scalar parameters to entry functions must have semantic annotations, and 
all
+struct parameters must have semantic annotations on every field in the struct
+declaration. Additionally if the entry function has a return type, a semantic
+annotation must be provided for the return type as well.
+
+HLSL entry functions can be called from other parts of the shader, which has
+implications on code generation.
+
+Implementation Details
+==
+
+In Clang, the DXC ``/E`` option is translated to the cc1 flag ``-hlsl-entry``,
+which in turn applies the ``HLSLShader`` attribute to the function with the
+specified name. This allows code generation for entry functions to always key
+off the presence of the ``HLSLShader`` attribute, regardless of what shader
+profile you are compiling.
+
+In code generation, two functions are generated. One is the user defined
+function, which is code generated as a mangled C++ function with internal
+linkage following normal function code generation.
+
+The actual exported entry function which can be called by the GPU driver is a
+``void(void)`` function that isn't name mangled. In code generation we generate
+the unmangled entry function, instantiations of the parameters with their
+semantic values populated, and a call to the user-defined function. After the
+call instruction the return value (if any) is saved using a target-appropriate
+intrinsic for storing outputs (for DirectX, the ``llvm.dx.store.output``).
+
+.. note::
+
+   HLSL support in Clang is currently focused on compute shaders, which do not
+   support output semantics. Support for output semantics will not be
+   implemented until other shader profiles are supported.
+
+Below is example IR that represents the planned implementation, subject to
+change as the ``llvm.dx.store.output`` and ``llvm.dx.load.input`` intrinsics 
are
+not yet implemented.
+
+.. code-block:: none
+
+   ; Function Attrs: norecurse
+   define void @main() #1 {
+  entry:
+  %0 = call i32 @llvm.dx.load.input.i32(...)
+  %1 = call i32 @"?main@@YAXII@Z"(i32 %0)
+  call @llvm.dx.store.output.i32(%1, ...)
+  ret void
+   }
+


Index: clang/docs/HLSL/HLSLDocs.rst
===
--- clang/docs/HLSL/HLSLDocs.rst
+++ clang/docs/HLSL/HLSLDocs.rst
@@ -12,3 +12,4 @@
:maxdepth: 1
 
ResourceTypes
+   EntryFunctions
Index: clang/docs/HLSL/EntryFunctions.rst
===
--- /dev/null
+++ clang/docs/HLSL/EntryFunctions.rst
@@ -0,0 +1,65 @@
+
+HLSL Entry Functions
+
+
+.. contents::
+   :local:
+
+Usage
+=
+
+In HLSL, entry functions denote the starting point for shader execution. They
+must be known at compile time. For all non-library shaders, the compiler assumes
+the default entry function name ``main``, unless the DXC ``/E`` option is
+provided to specify an alternate entry point. For library shaders entry points
+are denoted using the ``[shader(...)]`` attribute.
+
+All scalar parameters to entry functions must have semantic annotations, and all
+struct parameters must have semantic annotations on every field in the struct
+declaration. Additionally if the entry function has a return type, a semantic
+annotation must be provided for the return type as well.
+
+HLSL entry functions can be called from other parts of the shader, which has
+implications on code generation.
+
+Implementation Details
+==
+
+In Clang, the DXC ``/E`` option is translated to the cc1 flag ``-hlsl-entry``,
+which in turn applies the ``HLSLShader`` attribute to the function with the

[clang] 739a747 - [Docs] [HLSL] Documenting HLSL Entry Functions

[Chris Bieneman via cfe-commits]

Author: Chris Bieneman
Date: 2022-08-30T12:18:44-05:00
New Revision: 739a747b2368652155ac78f0ac341a6bfe640c60

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

LOG: [Docs] [HLSL] Documenting HLSL Entry Functions

This document describes the basic usage and implementation details for
HLSL entry functions in Clang.

Reviewed By: python3kgae

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

Added: 
clang/docs/HLSL/EntryFunctions.rst

Modified: 
clang/docs/HLSL/HLSLDocs.rst

Removed: 




diff  --git a/clang/docs/HLSL/EntryFunctions.rst 
b/clang/docs/HLSL/EntryFunctions.rst
new file mode 100644
index ..0e547aab420c
--- /dev/null
+++ b/clang/docs/HLSL/EntryFunctions.rst
@@ -0,0 +1,65 @@
+
+HLSL Entry Functions
+
+
+.. contents::
+   :local:
+
+Usage
+=
+
+In HLSL, entry functions denote the starting point for shader execution. They
+must be known at compile time. For all non-library shaders, the compiler 
assumes
+the default entry function name ``main``, unless the DXC ``/E`` option is
+provided to specify an alternate entry point. For library shaders entry points
+are denoted using the ``[shader(...)]`` attribute.
+
+All scalar parameters to entry functions must have semantic annotations, and 
all
+struct parameters must have semantic annotations on every field in the struct
+declaration. Additionally if the entry function has a return type, a semantic
+annotation must be provided for the return type as well.
+
+HLSL entry functions can be called from other parts of the shader, which has
+implications on code generation.
+
+Implementation Details
+==
+
+In Clang, the DXC ``/E`` option is translated to the cc1 flag ``-hlsl-entry``,
+which in turn applies the ``HLSLShader`` attribute to the function with the
+specified name. This allows code generation for entry functions to always key
+off the presence of the ``HLSLShader`` attribute, regardless of what shader
+profile you are compiling.
+
+In code generation, two functions are generated. One is the user defined
+function, which is code generated as a mangled C++ function with internal
+linkage following normal function code generation.
+
+The actual exported entry function which can be called by the GPU driver is a
+``void(void)`` function that isn't name mangled. In code generation we generate
+the unmangled entry function, instantiations of the parameters with their
+semantic values populated, and a call to the user-defined function. After the
+call instruction the return value (if any) is saved using a target-appropriate
+intrinsic for storing outputs (for DirectX, the ``llvm.dx.store.output``).
+
+.. note::
+
+   HLSL support in Clang is currently focused on compute shaders, which do not
+   support output semantics. Support for output semantics will not be
+   implemented until other shader profiles are supported.
+
+Below is example IR that represents the planned implementation, subject to
+change as the ``llvm.dx.store.output`` and ``llvm.dx.load.input`` intrinsics 
are
+not yet implemented.
+
+.. code-block:: none
+
+   ; Function Attrs: norecurse
+   define void @main() #1 {
+  entry:
+  %0 = call i32 @llvm.dx.load.input.i32(...)
+  %1 = call i32 @"?main@@YAXII@Z"(i32 %0)
+  call @llvm.dx.store.output.i32(%1, ...)
+  ret void
+   }
+

diff  --git a/clang/docs/HLSL/HLSLDocs.rst b/clang/docs/HLSL/HLSLDocs.rst
index 0b66e517a73a..2cfe631da939 100644
--- a/clang/docs/HLSL/HLSLDocs.rst
+++ b/clang/docs/HLSL/HLSLDocs.rst
@@ -12,3 +12,4 @@ HLSL Design and Implementation
:maxdepth: 1
 
ResourceTypes
+   EntryFunctions



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

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 456710.
tejohnson marked 5 inline comments as done.
tejohnson added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128142

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/Inputs/memprof.exe
  clang/test/CodeGen/Inputs/memprof.memprofraw
  clang/test/CodeGen/memprof.cpp
  llvm/include/llvm/Analysis/MemoryBuiltins.h
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/lib/Analysis/MemoryBuiltins.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/memprof.exe
  llvm/test/Transforms/PGOProfile/Inputs/memprof.memprofraw
  llvm/test/Transforms/PGOProfile/Inputs/memprof_pgo.profraw
  llvm/test/Transforms/PGOProfile/memprof.ll
  llvm/test/Transforms/PGOProfile/memprofmissingfunc.ll

Index: llvm/test/Transforms/PGOProfile/memprofmissingfunc.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/memprofmissingfunc.ll
@@ -0,0 +1,25 @@
+;; Tests that we get a missing memprof error for a function not in profile when
+;; using -pgo-warn-missing-function.
+
+;; TODO: Use text profile inputs once that is available for memprof.
+
+;; The raw profiles have been generated from the source used for the memprof.ll
+;; test (see comments at the top of that file).
+
+; RUN: llvm-profdata merge %S/Inputs/memprof.memprofraw --profiled-binary %S/Inputs/memprof.exe -o %t.memprofdata
+
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.memprofdata -pgo-warn-missing-function -S 2>&1 | FileCheck %s
+
+; CHECK: memprof record not found for function hash {{.*}} _Z16funcnotinprofilev
+
+; ModuleID = 'memprofmissingfunc.cc'
+source_filename = "memprofmissingfunc.cc"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: mustprogress noinline nounwind optnone uwtable
+define dso_local void @_Z16funcnotinprofilev() {
+entry:
+  ret void
+}
+
Index: llvm/test/Transforms/PGOProfile/memprof.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/memprof.ll
@@ -0,0 +1,483 @@
+;; Tests memprof profile matching (with and without instrumentation profiles).
+
+;; TODO: Use text profile inputs once that is available for memprof.
+
+;; The input IR and raw profiles have been generated from the following source:
+;;
+;; #include 
+;; #include 
+;; #include 
+;; char *foo() {
+;;   return new char[10];
+;; }
+;; char *foo2() {
+;;   return foo();
+;; }
+;; char *bar() {
+;;   return foo2();
+;; }
+;; char *baz() {
+;;   return foo2();
+;; }
+;; char *recurse(unsigned n) {
+;;   if (!n)
+;; return foo();
+;;   return recurse(n-1);
+;; }
+;; int main(int argc, char **argv) {
+;;   // Test allocations with different combinations of stack contexts and
+;;   // coldness (based on lifetime, since they are all accessed a single time
+;;   // per byte via the memset).
+;;   char *a = new char[10];
+;;   char *b = new char[10];
+;;   char *c = foo();
+;;   char *d = foo();
+;;   char *e = bar();
+;;   char *f = baz();
+;;   memset(a, 0, 10);
+;;   memset(b, 0, 10);
+;;   memset(c, 0, 10);
+;;   memset(d, 0, 10);
+;;   memset(e, 0, 10);
+;;   memset(f, 0, 10);
+;;   // a and c have short lifetimes
+;;   delete[] a;
+;;   delete[] c;
+;;   // b, d, e, and f have long lifetimes and will be detected as cold by default.
+;;   sleep(200);
+;;   delete[] b;
+;;   delete[] d;
+;;   delete[] e;
+;;   delete[] f;
+;;   // Loop ensures the two calls to recurse have stack contexts that only differ
+;;   // in one level of recursion. We should get two stack contexts reflecting the
+;;   // different levels of recursion and different allocation behavior (since the
+;;   // first has a very long lifetime and the second has a short lifetime).
+;;   for (unsigned i = 0; i < 2; i++) {
+;; char *g = recurse(i + 3);
+;; memset(g, 0, 10);
+;; if (!i)
+;;   sleep(200);
+;; delete[] g;
+;;   }
+;;   return 0;
+;; }
+;;
+;; The following commands were used to compile the source to instrumented
+;; executables and collect raw binary format profiles:
+;;
+;; # Collect memory profile:
+;; $ clang++ -fuse-ld=lld -no-pie -Wl,--no-rosegment -gmlt \
+;; 	-fdebug-info-for-profiling -mno-omit-leaf-frame-pointer \
+;;	-fno-omit-frame-pointer -fno-optimize-sibling-calls -m64 -Wl,-build-id \
+;; 	memprof.cc -o memprof.exe -fmemory-profile
+;; $ env MEMPROF_OPTIONS=log_path=stdout ./memprof.exe > memprof.memprofraw
+;;
+;; # Collect IR PGO profile:
+;; $ clang++ -fuse-ld=lld -no-pie -Wl,--no-rosegment -gmlt \
+;; 	-fdebug-info-for-profiling -mno-omit-leaf-frame-pointer \
+;;	-fno-omit-frame-pointer -fno-optimize-sibling-calls -m64 -Wl,-build-id \
+;; 	memprof.cc -o pgo.exe -fprofile-generate=.
+;; 

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson marked 12 inline comments as done.
tejohnson added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1252
+AllocInfo->Info.getMinLifetime());
+  SmallVector StackIds;
+  std::set StackHashSet;

snehasish wrote:
> I think if you use an llvm::SetVector here instead then you don't need the 
> StackHashSet std::set below. CallstackTrie::addCallstack already accepts an 
> ArrayRef so it won't need to change if we use a SetVector.
Noted, but moot as this has been removed (see below)



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1253
+  SmallVector StackIds;
+  std::set StackHashSet;
+  for (auto StackFrame : AllocInfo->CallStack) {

snehasish wrote:
> nit: It doesn't look like we #include  in this file so we are probably 
> relying on having it transitively being included from somewhere.
Although this std::set has been removed, I use it elsewhere so added the 
include.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1256
+auto StackId = computeStackId(StackFrame);
+// Remove recursion cycles.
+// TODO: Consider handling this during profile generation.

I have removed this handling. It is not correct in the case of mutual recursion 
involving more than one function where it will result in non-sensical stack 
traces. I am deferring handling recursion until later during the LTO phase.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1276
+  if (Error E = MemProfResult.takeError()) {
+handleAllErrors(std::move(E), [&](const InstrProfError ) {
+  auto Err = IPE.get();

snehasish wrote:
> Consider defining the lambda outside above the condition to reduce 
> indentation. IMO it will be  a little easier to follow if it wasn't inlined 
> into the if statement itself.
I could do this, but as is it mirrors the structure of the similar handling in 
readCounters, which has some advantages. wdyt?



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1313
+  std::map *, unsigned>>>
+  LocHashToCallSites;
+  const auto MemProfRec = std::move(MemProfResult.get());

snehasish wrote:
> LocHashToCallSiteFrame to indicate the value in the map corresponds to an 
> individual frame?
The key is the location hash (stack id) of a single frame, but the value is a 
set of all the CallSites from the profile that reference it.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1330
+  // Once we find this function, we can stop recording.
+  if (StackFrame.Function == FuncGUID)
+break;

snehasish wrote:
> Should we assert that it was actually found?
Added assert after the loop.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1360
+  // If leaf was found in a map, iterators pointing to its location in both
+  // of the maps (it may only exist in one).
+  std::map>::iterator

snehasish wrote:
> Can you add an assert for this?
In this case "may only" meant "might only", not "may only at most". So I can't 
assert on anything. This can happen for example if we have a location that 
corresponds to both an allocation call and another callsite (I've seen this 
periodically, and can reproduce e.g. with a macro). We would need to use 
discriminators more widely to better distinguish them in that case (with the 
handling here we will only match to the allocation call for now - edit: a 
slight change noted further below ensures this is the case). Will change 
/may/might/ and add a note.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1414
+
+  // First add !memprof metadata from allocation info, if we found the
+  // instruction's leaf location in that map, and if the rest of the

snehasish wrote:
> "First add !memprof metadata  ..." -- the ordering of the if-else condition 
> isn't necessary though since only one of the iters can be non-null? We could 
> rewrite the else condition first to reduce the complexity here a bit. Eg --
> 
> ```
> if (CallSitesIter != LocHashToCallSites.end()) {
>   ...
>   continue
> }
> 
> // Flip the conditions here
> if (!isNewLikeFn() || AllocInfoIter == LocHashToAllocInfo.end()) {
>continue
> }
> 
> CallStackTrie AllocTrie;
> ...
> ```
As noted earlier, it might be in more than one map. But I realized we could 
sometimes add the callsite metadata, instead of the memprof metadata, to a 
non-new allocation call (e.g. malloc) when there is a matching location in both 
maps given the structuring of the handling below. I've changed it so we handle 
all instructions with matching allocation profile data in the below if 
statement, and skip adding any metadata 

[PATCH] D132421: [HLSL] Support PCH for cc1 mode

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/include/clang/Sema/HLSLExternalSemaSource.h:58
+/// them before we initialize the ExternalSemaSource base class.
+struct ChainedHLSLExternalSemaSourceMembers {
+  ChainedHLSLExternalSemaSourceMembers(ExternalSemaSource *ExtSema)

IIUC, this code just exists to make sure that the `ASTReader` deserializes 
before the external sema source starts adding things. Is that correct?

If so, you don't need to do this, instead you can just add this code to 
`InitializeSema()` to force the `ASTReader` to de-serialize the decls:

```
// If the translation unit has external storage force external decls to load.
if (AST.getTranslationUnitDecl()->hasExternalLexicalStorage())
(void)AST.getTranslationUnitDecl()->decls_begin();
```



Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:297
+  }
+
   IdentifierInfo  = AST.Idents.get("hlsl", tok::TokenKind::identifier);

I think the core of what you'e doing here is not far off, but I think this 
pattern doesn't scale or cover all the use cases that matter.

The cases that come to my mind are:
1) PCH has a forward declaration, source doesn't use the type so we shouldn't 
complete the type.
2) PCH has a forward declaration, source uses the type, so we need to generate 
a complete decl.
3) PCH has a complete declaration, so we should re-use the declaration and not 
need to do anything.

We also need to keep in mind that we're going to keep adding more and more 
types, so this pattern where each new type needs to add code to lookup is going 
to be a challenge. It is also going to be a challenge to manage the complexity 
of PCH defines one thing but not another.

One thing you should look at is how clang handles the following C++ code:

```
template
class Foo;

template
class Foo {
  T Val;
};
```

This results in two decls. One for the forward decl and the second for the 
definition. The definition lists the forward declaration as the previous decl. 
We should do that here.

When we generate the HLSL namespace, we can do a lookup and if we find a 
previous decl, set the previous decl.

When we generate any of the builtin types, we can lookup the previous decl, and 
annotate as the previous decl. We'll can also handle the case where a decl from 
the PCH is complete. If the PCH provides a complete decl, we can skip 
generating any decls for it, and the lookups should just work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132421

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

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> So I think the most valuable optimizations are low-level optimizations to 
> `ImmutableMap`. There were a few suggestions on the mailing list to use 
> something more modern than the AVL trees under the hood but I don't think 
> authors found much success with those.

Back then I was experimenting 

 by replacing some of our `ImmutableMap` instances with an alternative 
implementation. I've started with `Environment::ExprBindings` because profiling 
showed that as the most frequently used. I've chosen `immer::map` as an 
alternative immutable map implementation. It seemed to be very promising 
because of

- the applied Relaxed Radix Balanced Tree implementation 

- efficient batch manipulations 
 .

Results: I could make the LIT tests pass, except two checks. The performance 
was not promising though. The run-time was similar that we had with the AVL 
trees, however, the memory consumption was 5-10x higher! Consequently, I've 
abandoned the prototype. But, I think an **//efficient batch manipulation 
implementation with the existing AVL based//** `ImmutableMap` might be worth to 
experiment with further.

> - From high-level perspective almost half the time is spent in mark-and-sweep 
> garbage collection in `ExprEngine::removeDead()`. Which can't really be cut 
> as running `removeDead()` less often only increases the analysis time as it 
> makes the maps larger and therefore slower to access. And it's also hard to 
> optimize because the procedure is very complicated and fragile and very few 
> people actually understand how it works.

Yes, this is also aligned with my measurements. There are analyzer options to 
manipulate when the `removeDead` is called, before every Stmt, or before every 
basic block, or never. Actually, the last option is meaningless, because 
renders both the engine and many checkers faulty. The `before every basic 
block` option might still be worth to experiment with, but I could not find 
reasonable arguments to change to that yet (I cannot recall, but some lit tests 
were failing miserably with that option too).


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

https://reviews.llvm.org/D131707

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

[PATCH] D132932: [Clang][Comments] Parse `` in doc comments correctly

[Egor Zhdan via Phabricator via cfe-commits]

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



Comment at: clang/tools/libclang/CXString.cpp:85
+  if (String.empty())
+return createEmpty();
+

gribozavr2 wrote:
> Please split this change into a separate patch and add a unit test.
> 
Do you know a good way to add a unit-test for this? This function only gets 
called from within libclang, it isn't exposed to clients of libclang, including 
libclangTest. So I think a unit test would probably need to invoke the same 
CXComment API that `c-index-test` invokes while running 
`comment-to-html-xml-conversion.cpp`. Would that be worth it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132932

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

[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

[Matheus Izvekov via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaTemplateVariadic.cpp:860
+} else if (const auto *ND = Unexpanded[I].first.get();
+   isa(ND)) {
+  // Function parameter pack or init-capture pack.

erichkeane wrote:
> mizvekov wrote:
> > mizvekov wrote:
> > > erichkeane wrote:
> > > > This pattern with the init + condition is a little awkward here... any 
> > > > reason to not just use the cast around the 'ND" and just use the VD in 
> > > > the use below? or is there a good reason to split it up like this?
> > > > 
> > > > Same with the above case.
> > > No very strong reason than that just from my different perception this 
> > > looked fine :)
> > > 
> > > Minor advantage that we don't make the variable a VarDecl pointer if we 
> > > don't need to access it as such.
> > > 
> > > But no strong preference here, I can have another look tomorrow.
> > I played a little bit with this change.
> > 
> > I think one thing that makes that pattern of adding separate ND and VD a 
> > bit confusing is that at a glance it almost looks like these are different 
> > cases in the PointerUnion variant. You have to do a double take to see 
> > that, since the nested cast is a bit subtle. This can become even subtler 
> > as we add more cases in the next patch.
> > 
> > Or we could stop using PointerUnion on the next patch, since it's so 
> > unergonomic with so many variants.
> > 
> > Anyway, I did some other refactorings and I think in general this will be 
> > much clearer to read on my next push.
> > 
> > With this refactoring, on this part here that problem goes away since we 
> > make this section produce a NamedDecl.
> > 
> > On the second part, below, I tidied up the code so much that I think that 
> > nested else isn't almost invisible anymore, since the other blocks become 
> > about the same size.
> > 
> > Otherwise, let me know what you think.
> > 
> > I also added to this first part here a FIXME comment describing a 
> > pre-existing problem where if we get a canonical TTP, the diagnostics fail 
> > to point to the relevant parameter since we won't have it's identifier.
> > 
> > Will try to add a repro and fix for that on the next patch.
> Thanks for spending time on this... the nested conditionals on the var-decl 
> was hiding 90%+ of what was going on in the branch, and at least this top one 
> is BETTER enough than before.  I too hate the pointer union (note BTW, that 
> _4_ is the maximum number of elements thanks to 32 bit platforms, that'll 
> burn you :)).
> 
> 
Thanks, true about the maximum size :)

Some of the cases could be unified as we have two types and two expressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128095

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

[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

[Matheus Izvekov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3a0309c53674: [clang] Improve diagnostics for expansion 
length mismatch (authored by mizvekov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128095

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaInternal.h
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
  clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
  clang/test/SemaTemplate/pack-deduction.cpp

Index: clang/test/SemaTemplate/pack-deduction.cpp
===
--- clang/test/SemaTemplate/pack-deduction.cpp
+++ clang/test/SemaTemplate/pack-deduction.cpp
@@ -134,14 +134,14 @@
   template struct tuple {};
   template struct A {
 template static pair, tuple> f(pair ...p);
-// expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 3) from outer parameter packs}}
+// expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 3)}}
 // expected-note@-2 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (at least 3 vs. 2) from outer parameter packs}}
 
 template static pair, tuple> g(pair ...p, ...);
-// expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 3) from outer parameter packs}}
+// expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 3)}}
 
 template static tuple h(tuple..., pair>);
-// expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 1) from outer parameter packs}}
+// expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 1)}}
   };
 
   pair, tuple> k1 = A().f(pair(), pair());
Index: clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
===
--- clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
+++ clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
@@ -97,7 +97,7 @@
   template  struct Constant {};
 
   template  struct Sum {
-template  using type = Constant<((Is + Js) + ... + 0)>; // expected-error {{pack expansion contains parameter pack 'Js' that has a different length (1 vs. 2) from outer parameter packs}}
+template  using type = Constant<((Is + Js) + ... + 0)>; // expected-error {{pack expansion contains parameter packs 'Is' and 'Js' that have different lengths (1 vs. 2)}}
   };
 
   Sum<1>::type<1, 2> x; // expected-note {{instantiation of}}
Index: clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -473,7 +473,7 @@
 namespace pr56094 {
 template  struct D {
   template  using B = int(int (*...p)(T, U));
-  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a different length (1 vs. 2) from outer parameter packs}}
+  // expected-error@-1 {{pack expansion contains parameter packs 'T' and 'U' that have different lengths (1 vs. 2)}}
   template  D(B *);
   // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
 };
@@ -484,7 +484,7 @@
 template  struct G {};
 template  struct E {
   template  using B = G...>;
-  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a different length (1 vs. 2) from outer parameter packs}}
+  // expected-error@-1 {{pack expansion contains parameter packs 'I' and 'U' that have different lengths (1 vs. 2)}}
   template  E(B *);
   // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
 };
Index: clang/lib/Sema/SemaTemplateVariadic.cpp
===
--- clang/lib/Sema/SemaTemplateVariadic.cpp
+++ clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -88,6 +88,23 @@
   return true;
 }
 
+bool
+VisitSubstTemplateTypeParmPackTypeLoc(SubstTemplateTypeParmPackTypeLoc TL) {
+  Unexpanded.push_back({TL.getTypePtr(), TL.getNameLoc()});
+  return true;
+}
+
+bool VisitSubstTemplateTypeParmPackType(SubstTemplateTypeParmPackType *T) {
+  Unexpanded.push_back({T, SourceLocation()});
+  return true;
+}
+
+bool
+VisitSubstNonTypeTemplateParmPackExpr(SubstNonTypeTemplateParmPackExpr *E) {
+  Unexpanded.push_back({E, E->getParameterPackLocation()});
+  return true;
+}
+
 /// Record occurrences of function and non-type template
 /// parameter packs in an expression.

[clang] 3a0309c - [clang] Improve diagnostics for expansion length mismatch

[Matheus Izvekov via cfe-commits]

Author: Matheus Izvekov
Date: 2022-08-30T18:58:38+02:00
New Revision: 3a0309c53674be56b5cfce038d78a0c2c6e2a98c

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

LOG: [clang] Improve diagnostics for expansion length mismatch

When checking parameter packs for expansion, instead of basing the diagnostic 
for
length mismatch for outer parameters only on the known number of expansions,
we should also analyze SubstTemplateTypeParmPackType and 
SubstNonTypeTemplateParmPackExpr
for unexpanded packs, so we can emit a diagnostic pointing to a concrete
outer parameter.

Signed-off-by: Matheus Izvekov 

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/include/clang/Sema/SemaInternal.h
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/lib/Sema/SemaTemplateVariadic.cpp
clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
clang/test/SemaTemplate/pack-deduction.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 132e282797053..824ab42706334 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -118,6 +118,8 @@ Improvements to Clang's diagnostics
 - Correctly diagnose a future keyword if it exist as a keyword in the higher
   language version and specifies in which version it will be a keyword. This
   supports both c and c++ language.
+- When diagnosing multi-level pack expansions of mismatched lengths, Clang will
+  now, in most cases, be able to point to the relevant outer parameter.
 
 Non-comprehensive list of changes in this release
 -

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 0ee21cf64aa34..bd81d3a7e5ee5 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -238,8 +238,11 @@ namespace threadSafety {
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair,
-  SourceLocation> UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+llvm::PointerUnion<
+const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.

diff  --git a/clang/include/clang/Sema/SemaInternal.h 
b/clang/include/clang/Sema/SemaInternal.h
index 842eec099540c..4eca50919dc7e 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -62,7 +62,7 @@ inline InheritableAttr *getDLLAttr(Decl *D) {
 }
 
 /// Retrieve the depth and index of a template parameter.
-inline std::pair getDepthAndIndex(NamedDecl *ND) {
+inline std::pair getDepthAndIndex(const NamedDecl *ND) {
   if (const auto *TTP = dyn_cast(ND))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
@@ -79,7 +79,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = UPP.first.dyn_cast())
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(UPP.first.get());
+  return getDepthAndIndex(UPP.first.get());
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {

diff  --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 84c4a191ec327..e8837d88f97dd 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -738,8 +738,11 @@ class PackDeductionScope {
   SmallVector Unexpanded;
   S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-unsigned Depth, Index;
-std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+UnexpandedParameterPack U = Unexpanded[I];
+if (U.first.is() ||
+U.first.is())
+  continue;
+auto [Depth, Index] = getDepthAndIndex(U);
 if (Depth == Info.getDeducedDepth())
   AddPack(Index);
   }

diff  --git a/clang/lib/Sema/SemaTemplateVariadic.cpp 
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 285bf67398f16..69e0c70bbec53 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -88,6 +88,23 @@ namespace {
   return true;
 }
 
+bool
+VisitSubstTemplateTypeParmPackTypeLoc(SubstTemplateTypeParmPackTypeLoc TL) 
{
+  Unexpanded.push_back({TL.getTypePtr(), TL.getNameLoc()});
+  return true;
+}
+
+bool 

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

Another test under llvm-project/clang/test/CodeGen/ is needed which tests IR 
generated from C/C++ contains expected info.
Random example clang/test/CodeGen/attr-noundef.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130888

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

[PATCH] D131388: [docs] Add "Standard C++ Modules"

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

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

I just noticed that there probably needs to be one change. The doc sometimes 
describes `-fprebuilt-module-interface`, which is not a valid option. I think 
the occurrences were meant to be `-fpbrebuilt-module-path`.


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

https://reviews.llvm.org/D131388

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

[PATCH] D132352: Introduce noread_thread_id to address the thread identification problem in coroutines

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D132352#3757433 , @ChuanqiXu wrote:

> In D132352#3757415 , @rjmccall 
> wrote:
>
>> Stackful coroutine bodies should be straightforward to support on top of the 
>> other work you've been doing, if anyone's actually interested in pursuing 
>> them.  As far as the optimizer needs to know, a stackful coroutine function 
>> is just like a presplit stackless coroutine except that calls and returns 
>> work normally and it's never split.  Because it's never split, the backends 
>> would need to understand that they can't arbitrarily reorder TLS 
>> materializations and so on in those functions, which would probably be the 
>> most complicated piece of work there.  Otherwise, I think we'd just need to 
>> mark stackful coroutine bodies with some new attribute and then change 
>> `cannotReadDifferentThreadIDIfMoved` to check for that, the same way it 
>> checks for presplit stackless coroutines.
>
> As far as I understand, we can't mark stackful coroutine bodies with special 
> attributes. It is slightly different from stackless coroutine. A stackless 
> coroutine is a suspendable function. So we can mark the function. But the 
> stackful coroutine is a thread in the user space actually. (Or we can think 
> stackful coroutine as a stack instead of a function)  In another word, if a 
> function A in a stackful coroutine calls another function B, then B lives in 
> the stackful coroutine too. The stackful coroutine switches by user(library) 
> implemented methods and they are not standardized so we can't even do hacks 
> for them.

Well, it's complicated.  Stackful coroutines don't generally involve creating a 
true thread, just a stack, and then yes, you swap the stack on the current 
thread in some system-specific way.  What I'm pointing out is that stackful 
coroutines can therefore observe changes in their thread ID across suspension 
points, which is the exact same problem as stackless coroutines have.  So we 
actually *would* need them to be identified specially, even if they're 
otherwise straightforward to compile, just so that we understand those special 
semantics and don't e.g. miscompile TLV accesses by hoisting them over a 
suspension.  (That this is still necessary might perhaps undermine some of the 
arguments for pursuing stackful coroutines in the first place; nonetheless, 
it's true.)

> I don't pursue stackful coroutine too. I raise the example to show the idea 
> of `noread_thread_id` may have some slight advantage.

Sure, we don't need to talk about this any further, since nobody's interested 
in pursuing it right now.  I'm just trying to underline the connections to what 
we've already done.


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

https://reviews.llvm.org/D132352

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