[PATCH] D133589: [clang-format] JSON formatting add new option for controlling newlines in json arrays

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 459408.
MyDeveloperDay added a comment.

- address review comments


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

https://reviews.llvm.org/D133589

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestJson.cpp

Index: clang/unittests/Format/FormatTestJson.cpp
===
--- clang/unittests/Format/FormatTestJson.cpp
+++ clang/unittests/Format/FormatTestJson.cpp
@@ -159,6 +159,27 @@
"]");
 }
 
+TEST_F(FormatTestJson, JsonArrayOneLine) {
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
+  Style.BreakArrays = false;
+  Style.SpacesInContainerLiterals = false;
+  verifyFormat("[]", Style);
+  verifyFormat("[1]", Style);
+  verifyFormat("[1, 2]", Style);
+  verifyFormat("[1, 2, 3]", Style);
+  verifyFormat("[1, 2, 3, 4]", Style);
+  verifyFormat("[1, 2, 3, 4, 5]", Style);
+
+  verifyFormat("[\n"
+   "  1,\n"
+   "  2,\n"
+   "  {\n"
+   "A: 1\n"
+   "  }\n"
+   "]",
+   Style);
+}
+
 TEST_F(FormatTestJson, JsonNoStringSplit) {
   FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
   Style.IndentWidth = 4;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4399,18 +4399,22 @@
 // }
 if (Left.is(TT_DictLiteral) && Left.is(tok::l_brace))
   return true;
-// Always break after a JSON array opener.
-// [
-// ]
+// Always break after a JSON array opener based on BreakArrays.
 if (Left.is(TT_ArrayInitializerLSquare) && Left.is(tok::l_square) &&
-!Right.is(tok::r_square)) {
-  return true;
+Right.isNot(tok::r_square) ||
+Left.is(tok::comma)) {
+  if (Right.is(tok::l_brace))
+return true;
+  // scan to the right if an we see an object or an array inside
+  // then break.
+  for (const auto *Tok = &Right; Tok; Tok = Tok->Next) {
+if (Tok->isOneOf(tok::l_brace, tok::l_square))
+  return true;
+if (Tok->isOneOf(tok::r_brace, tok::r_square))
+  break;
+  }
+  return Style.BreakArrays;
 }
-// Always break after successive entries.
-// 1,
-// 2
-if (Left.is(tok::comma))
-  return true;
   }
 
   // If the last token before a '}', ']', or ')' is a comma or a trailing
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -706,9 +706,8 @@
 // BreakInheritance was not, initialize the latter from the
 // former for backwards compatibility.
 if (BreakBeforeInheritanceComma &&
-Style.BreakInheritanceList == FormatStyle::BILS_BeforeColon) {
+Style.BreakInheritanceList == FormatStyle::BILS_BeforeColon)
   Style.BreakInheritanceList = FormatStyle::BILS_BeforeComma;
-}
 
 IO.mapOptional("BreakBeforeTernaryOperators",
Style.BreakBeforeTernaryOperators);
@@ -722,12 +721,12 @@
 // BreakConstructorInitializers was not, initialize the latter from the
 // former for backwards compatibility.
 if (BreakConstructorInitializersBeforeComma &&
-Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeColon) {
+Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeColon)
   Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma;
-}
 
 IO.mapOptional("BreakAfterJavaFieldAnnotations",
Style.BreakAfterJavaFieldAnnotations);
+IO.mapOptional("BreakArrays", Style.BreakArrays);
 IO.mapOptional("BreakStringLiterals", Style.BreakStringLiterals);
 IO.mapOptional("ColumnLimit", Style.ColumnLimit);
 IO.mapOptional("CommentPragmas", Style.CommentPragmas);
@@ -755,7 +754,6 @@
Style.EmptyLineBeforeAccessModifier);
 IO.mapOptional("ExperimentalAutoDetectBinPacking",
Style.ExperimentalAutoDetectBinPacking);
-
 IO.mapOptional("PackConstructorInitializers",
Style.PackConstructorInitializers);
 // For backward compatibility:
@@ -1249,6 +1247,7 @@
   LLVMStyle.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never;
   LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
   LLVMStyle.ExperimentalAutoDetectBinPacking = false;
+  LLVMStyle.BreakArrays = true;
   LLVMStyle.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
   LLVMStyle.FixNamespaceComments = true;
   LLVMStyle.ForEachMacros.push_back("foreach");
@@ -1963,9 +1962,8 @@
 (Style.JavaScriptQuotes == FormatStyle::JSQS_Single &&
  !Input.startswith("\"")) |

[PATCH] D133589: [clang-format] JSON formatting add new option for controlling newlines in json arrays

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 6 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/include/clang/Format/Format.h:2076
+  /// \version 16
+  bool BreakArrays;
+

HazardyKnusperkeks wrote:
> HazardyKnusperkeks wrote:
> > Please sort.
> What about `Leave`?
I'm going to say that at present the boolean is probably enough as JSON 
formatting is really to do one or the other, but of course this can progress 
easily into an enum when there is a use case


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

https://reviews.llvm.org/D133589

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

[PATCH] D133571: [clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Format/Format.h:469
+  /// Different styles for merging short case labels.
+  enum ShortCaseLabelStyle : int8_t {
+/// Never merge case code

HazardyKnusperkeks wrote:
> While we're at it, shouldn't there be a `Leave`? ;)
I think Leave is Never


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

https://reviews.llvm.org/D133571

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

[PATCH] D133589: [clang-format] JSON formatting add new option for controlling newlines in json arrays

[Owen Pan via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:4403-4404
+// Always break after a JSON array opener based on BreakArrays.
 if (Left.is(TT_ArrayInitializerLSquare) && Left.is(tok::l_square) &&
-!Right.is(tok::r_square)) {
-  return true;
+Right.isNot(tok::r_square) ||
+Left.is(tok::comma)) {

The compiler will warn without the extra pair of parentheses?



Comment at: clang/lib/Format/TokenAnnotator.cpp:4427
+if (Left.is(tok::comma)) {
+  if (Right.is(tok::l_brace)) {
+return true;

MyDeveloperDay wrote:
> curdeius wrote:
> > You can elide braces here.
> This was funny I had RemoveBracesLLVM on, but it didn't get rid of them 
> automatically, I wonder if this was my fault or if we are missing a case for 
> the RemoveBraces option, but thanks you be done now
@MyDeveloperDay I'm very curious about this. Can you open an issue if it can be 
reproduced?


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

https://reviews.llvm.org/D133589

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

[PATCH] D132643: [OpenMP] Extend lit test for parallel for simd construct

[Animesh Kumar via Phabricator via cfe-commits]

animeshk-amd added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132643

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

[PATCH] D133683: [c++] implements tentative DR1432 for partial ordering of function template

[Yuanfang Chen via Phabricator via cfe-commits]

ychen created this revision.
ychen added reviewers: aaron.ballman, erichkeane.
Herald added a project: All.
ychen requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

D128745  handled DR1432 for the partial 
ordering of partial specializations, but
missed the handling for the partial ordering of function templates. This patch
implements the latter. While at it, also simplies the previous implementation to
be more close the wording without funtional changes.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133683

Files:
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/drs/dr6xx.cpp

Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -1083,13 +1083,18 @@
   // Also see dr1395.
 
   namespace temp_func_order_example2 {
-template  struct A {};
-template  void f(U, A *p = 0); // expected-note {{candidate}}
+template  struct A {}; // expected-error 0-1{{C++11}}
+template  void e(A) = delete; // expected-error 0-2{{C++11}}
+template  void e(A);
+template  void f(U, A *p = 0) = delete; // expected-note {{candidate}} expected-error 0-1{{C++11}}
 template  int &f(U, A *p = 0); // expected-note {{candidate}}
 template  void g(T, T = T()); // expected-note {{candidate}}
 template  void g(T, U...); // expected-note {{candidate}} expected-error 0-1{{C++11}}
 void h() {
-  int &r = f(42, (A *)0);
+  A a;
+  int &r = f(42, &a);
+  A b;
+  e(b);
   f(42); // expected-error {{ambiguous}}
   g(42); // expected-error {{ambiguous}}
 }
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -5187,6 +5187,32 @@
   return FT1;
   }
 
+  // Consider this a fix for CWG1432. Similar to the fix for CWG1395.
+  bool ClangABICompat15 =
+  Context.getLangOpts().getClangABICompat() <= LangOptions::ClangABI::Ver15;
+  if (!ClangABICompat15) {
+for (int i = 0, e = std::min(NumParams1, NumParams2); i < e; ++i) {
+  QualType T1 = FD1->getParamDecl(i)->getType().getDesugaredType(Context);
+  QualType T2 = FD2->getParamDecl(i)->getType().getDesugaredType(Context);
+  auto *TST1 = dyn_cast(T1);
+  auto *TST2 = dyn_cast(T2);
+  if (TST1 && TST2) {
+unsigned NumArgs1 = TST1->getNumArgs();
+unsigned NumArgs2 = TST2->getNumArgs();
+bool IsPackExpansion1 =
+NumArgs1 && TST1->template_arguments().back().isPackExpansion();
+bool IsPackExpansion2 =
+NumArgs2 && TST2->template_arguments().back().isPackExpansion();
+if (NumArgs1 != NumArgs2 && IsPackExpansion1 != IsPackExpansion2) {
+  if (NumArgs1 > NumArgs2 && IsPackExpansion1)
+return FT2;
+  if (NumArgs1 < NumArgs2 && IsPackExpansion2)
+return FT1;
+}
+  }
+}
+  }
+
   return JudgeByConstraints();
 }
 
@@ -5422,31 +5448,29 @@
 return nullptr;
 
   if (Better1 && Better2) {
+// Consider this a fix for CWG1432. Similar to the fix for CWG1395.
 bool ClangABICompat15 = S.Context.getLangOpts().getClangABICompat() <=
 LangOptions::ClangABI::Ver15;
 if (!ClangABICompat15) {
-  // Consider this a fix for CWG1432. Similar to the fix for CWG1395.
   auto *TST1 = T1->castAs();
   auto *TST2 = T2->castAs();
-  if (TST1->getNumArgs()) {
-const TemplateArgument &TA1 = TST1->template_arguments().back();
-if (TA1.getKind() == TemplateArgument::Pack) {
-  assert(TST1->getNumArgs() == TST2->getNumArgs());
-  const TemplateArgument &TA2 = TST2->template_arguments().back();
-  assert(TA2.getKind() == TemplateArgument::Pack);
-  unsigned PackSize1 = TA1.pack_size();
-  unsigned PackSize2 = TA2.pack_size();
-  bool IsPackExpansion1 =
-  PackSize1 && TA1.pack_elements().back().isPackExpansion();
-  bool IsPackExpansion2 =
-  PackSize2 && TA2.pack_elements().back().isPackExpansion();
-  if (PackSize1 != PackSize2 && IsPackExpansion1 != IsPackExpansion2) {
-if (PackSize1 > PackSize2 && IsPackExpansion1)
-  return GetP2()(P1, P2);
-if (PackSize1 < PackSize2 && IsPackExpansion2)
-  return P1;
-  }
-}
+  const TemplateArgument &TA1 = TST1->template_arguments().back();
+  const TemplateArgument &TA2 = TST2->template_arguments().back();
+  unsigned PackSize1 =
+  TA1.getKind() == TemplateArgument::Pack ? TA1.pack_size() : 0u;
+  unsigned PackSize2 =
+  TA2.getKind() == TemplateArgument::Pack ? TA2.pack_size() : 0u

[PATCH] D133635: [clang-format] Don't insert braces for loops with a null statement

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D133635#378 , @MyDeveloperDay 
wrote:

> Can't tell you how much I love this `InsertBraces` feature @owenpan,  I work 
> for a team (with a large code base) that uses them all the time, and we are 
> catching missing braces in decade-old code! Thank you

Glad to hear that `InsertBraces` is useful in the real world!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133635

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

[clang] 5889ed8 - [clang-format] Don't insert braces for loops with a null statement

[via cfe-commits]

Author: owenca
Date: 2022-09-12T00:37:47-07:00
New Revision: 5889ed83a0ffa71f2cf010b73bbbf210f9690778

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

LOG: [clang-format] Don't insert braces for loops with a null statement

This is a workaround for #57539.

Fixes #57509.

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 7ef1e82d754fe..699d2d37c71dc 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2605,8 +2605,7 @@ void UnwrappedLineParser::parseUnbracedBody(bool 
CheckEOF) {
   FormatToken *Tok = nullptr;
 
   if (Style.InsertBraces && !Line->InPPDirective && !Line->Tokens.empty() &&
-  PreprocessorDirectives.empty()) {
-assert(!Line->Tokens.empty());
+  PreprocessorDirectives.empty() && FormatTok->isNot(tok::semi)) {
 Tok = Style.BraceWrapping.AfterControlStatement == FormatStyle::BWACS_Never
   ? getLastNonComment(*Line)
   : Line->Tokens.back().Tok;

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index d3e657b018b77..aae300e67f62a 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -25365,6 +25365,30 @@ TEST_F(FormatTest, InsertBraces) {
"}",
Style);
 
+  verifyFormat("do {\n"
+   "#if 0\n"
+   " if (a) {\n"
+   "#else\n"
+   "  if (b) {\n"
+   "#endif\n"
+   "}\n"
+   "}\n"
+   "while (0)\n"
+   "  ;",
+   Style);
+  // TODO: Replace the test above with the one below after #57539 is fixed.
+#if 0
+  verifyFormat("do {\n"
+   "#if 0\n"
+   "  if (a) {\n"
+   "#else\n"
+   "  if (b) {\n"
+   "#endif\n"
+   "  }\n"
+   "} while (0);",
+   Style);
+#endif
+
   Style.ColumnLimit = 15;
 
   verifyFormat("#define A \\\n"



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

[PATCH] D133635: [clang-format] Don't insert braces for loops with a null statement

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5889ed83a0ff: [clang-format] Don't insert braces for 
loops with a null statement (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133635

Files:
  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
@@ -25365,6 +25365,30 @@
"}",
Style);
 
+  verifyFormat("do {\n"
+   "#if 0\n"
+   " if (a) {\n"
+   "#else\n"
+   "  if (b) {\n"
+   "#endif\n"
+   "}\n"
+   "}\n"
+   "while (0)\n"
+   "  ;",
+   Style);
+  // TODO: Replace the test above with the one below after #57539 is fixed.
+#if 0
+  verifyFormat("do {\n"
+   "#if 0\n"
+   "  if (a) {\n"
+   "#else\n"
+   "  if (b) {\n"
+   "#endif\n"
+   "  }\n"
+   "} while (0);",
+   Style);
+#endif
+
   Style.ColumnLimit = 15;
 
   verifyFormat("#define A \\\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2605,8 +2605,7 @@
   FormatToken *Tok = nullptr;
 
   if (Style.InsertBraces && !Line->InPPDirective && !Line->Tokens.empty() &&
-  PreprocessorDirectives.empty()) {
-assert(!Line->Tokens.empty());
+  PreprocessorDirectives.empty() && FormatTok->isNot(tok::semi)) {
 Tok = Style.BraceWrapping.AfterControlStatement == FormatStyle::BWACS_Never
   ? getLastNonComment(*Line)
   : Line->Tokens.back().Tok;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25365,6 +25365,30 @@
"}",
Style);
 
+  verifyFormat("do {\n"
+   "#if 0\n"
+   " if (a) {\n"
+   "#else\n"
+   "  if (b) {\n"
+   "#endif\n"
+   "}\n"
+   "}\n"
+   "while (0)\n"
+   "  ;",
+   Style);
+  // TODO: Replace the test above with the one below after #57539 is fixed.
+#if 0
+  verifyFormat("do {\n"
+   "#if 0\n"
+   "  if (a) {\n"
+   "#else\n"
+   "  if (b) {\n"
+   "#endif\n"
+   "  }\n"
+   "} while (0);",
+   Style);
+#endif
+
   Style.ColumnLimit = 15;
 
   verifyFormat("#define A \\\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2605,8 +2605,7 @@
   FormatToken *Tok = nullptr;
 
   if (Style.InsertBraces && !Line->InPPDirective && !Line->Tokens.empty() &&
-  PreprocessorDirectives.empty()) {
-assert(!Line->Tokens.empty());
+  PreprocessorDirectives.empty() && FormatTok->isNot(tok::semi)) {
 Tok = Style.BraceWrapping.AfterControlStatement == FormatStyle::BWACS_Never
   ? getLastNonComment(*Line)
   : Line->Tokens.back().Tok;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133659: [Clang] P1169R4: static operator()

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 459413.
royjacobson added a comment.

Fix test after warning text change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133659

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OperatorKinds.def
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/CXX/over/over.match/over.match.best/over.best.ics/p6.cpp
  clang/test/CXX/over/over.oper/p7.cpp
  clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
  clang/test/Parser/cxx2b-lambdas-ext-warns.cpp
  clang/test/Parser/cxx2b-lambdas.cpp
  clang/test/SemaCXX/overloaded-operator-decl.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1481,7 +1481,7 @@
 
   static operator()
   https://wg21.link/P1169R4";>P1169R4
-  No
+  Clang 16
 
 
   Extended floating-point types and standard names
Index: clang/test/SemaCXX/overloaded-operator-decl.cpp
===
--- clang/test/SemaCXX/overloaded-operator-decl.cpp
+++ clang/test/SemaCXX/overloaded-operator-decl.cpp
@@ -51,7 +51,7 @@
 
 namespace PR14120 {
   struct A {
-static void operator()(int& i) { ++i; } // expected-error{{overloaded 'operator()' cannot be a static member function}}
+static void operator()(int& i) { ++i; } // expected-error{{overloaded 'operator()' cannot be declared static}}
   };
   void f() {
 int i = 0;
Index: clang/test/Parser/cxx2b-lambdas.cpp
===
--- clang/test/Parser/cxx2b-lambdas.cpp
+++ clang/test/Parser/cxx2b-lambdas.cpp
@@ -38,3 +38,16 @@
 auto XL4 = [] requires true {}; // expected-error{{expected body}}
 auto XL5 = [] requires true requires true {}; // expected-error{{expected body}}
 auto XL6 = [] requires true noexcept requires true {}; // expected-error{{expected body}}
+
+auto XL7 = []() static static {}; // expected-error {{cannot appear multiple times}}
+auto XL8 = []() static mutable {}; // expected-error {{cannot be both mutable and static}}
+
+auto XL9 = [] static {};
+auto XL10 = []() static {};
+
+void static_captures() {
+  int x;
+  auto SC1 = [&]() static {}; // expected-error {{a static lambda cannot have any captures}}
+  auto SC2 = [&x]() static {}; // expected-error {{a static lambda cannot have any captures}}
+  auto SC3 = [=]() static {}; // expected-error {{a static lambda cannot have any captures}}
+}
Index: clang/test/Parser/cxx2b-lambdas-ext-warns.cpp
===
--- /dev/null
+++ clang/test/Parser/cxx2b-lambdas-ext-warns.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify=expected,cxx20
+// RUN: %clang_cc1 -std=c++2b %s -verify=expected,cxx2b
+
+//cxx2b-no-diagnostics
+
+auto L1 = [] constexpr {}; // cxx20-warning {{lambda without a parameter clause is a C++2b extension}}
+auto L2 = []() static {}; // cxx20-error {{static lambdas is a C++2b extension}}
+auto L3 = [] static {}; // cxx20-error {{static lambdas is a C++2b extension}} // cxx20-warning {{lambda without a parameter clause is a C++2b extension}}
Index: clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
@@ -0,0 +1,129 @@
+// RUN: %clang_cc1 -std=c++2b %s -emit-llvm -triple x86_64-linux -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++2b %s -emit-llvm -triple x86_64-windows-msvc -o - | FileCheck %s
+
+struct Functor {
+  static int operator()(int x, int y) {
+return x + y;
+  }
+};
+
+auto GetALambda() {
+  return [](int x, int y) static {
+return x + y;
+  };
+}
+
+void CallsTheLambda() {
+  GetALambda()(1, 2);
+}
+
+// CHECK:  define {{.*}}CallsTheLambda{{.*}}
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %call = call noundef i32 {{.*}}(i32 noundef 1, i32 noundef 2)
+// CHECK-NEXT:   ret void
+// CHECK-NEXT: }
+
+void call_static_call_operator() {
+  Functor f;
+  f(101, 102);
+  f.operator()(201, 202);
+  Functor{}(301, 302);
+}
+
+// CHECK:  define {{.*}}call_static_call_operator{{.*}}
+// CHECK-NEXT: entry:
+// CHECK:{{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 101, i32 noundef 102)
+// CHECK-NEXT:   {{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 201, i32 noundef 202)
+// CHECK-NEXT:   {{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 

[PATCH] D133659: [Clang] P1169R4: static operator()

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 459414.
royjacobson added a comment.

format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133659

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OperatorKinds.def
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/CXX/over/over.match/over.match.best/over.best.ics/p6.cpp
  clang/test/CXX/over/over.oper/p7.cpp
  clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
  clang/test/Parser/cxx2b-lambdas-ext-warns.cpp
  clang/test/Parser/cxx2b-lambdas.cpp
  clang/test/SemaCXX/overloaded-operator-decl.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1481,7 +1481,7 @@
 
   static operator()
   https://wg21.link/P1169R4";>P1169R4
-  No
+  Clang 16
 
 
   Extended floating-point types and standard names
Index: clang/test/SemaCXX/overloaded-operator-decl.cpp
===
--- clang/test/SemaCXX/overloaded-operator-decl.cpp
+++ clang/test/SemaCXX/overloaded-operator-decl.cpp
@@ -51,7 +51,7 @@
 
 namespace PR14120 {
   struct A {
-static void operator()(int& i) { ++i; } // expected-error{{overloaded 'operator()' cannot be a static member function}}
+static void operator()(int& i) { ++i; } // expected-error{{overloaded 'operator()' cannot be declared static}}
   };
   void f() {
 int i = 0;
Index: clang/test/Parser/cxx2b-lambdas.cpp
===
--- clang/test/Parser/cxx2b-lambdas.cpp
+++ clang/test/Parser/cxx2b-lambdas.cpp
@@ -38,3 +38,16 @@
 auto XL4 = [] requires true {}; // expected-error{{expected body}}
 auto XL5 = [] requires true requires true {}; // expected-error{{expected body}}
 auto XL6 = [] requires true noexcept requires true {}; // expected-error{{expected body}}
+
+auto XL7 = []() static static {}; // expected-error {{cannot appear multiple times}}
+auto XL8 = []() static mutable {}; // expected-error {{cannot be both mutable and static}}
+
+auto XL9 = [] static {};
+auto XL10 = []() static {};
+
+void static_captures() {
+  int x;
+  auto SC1 = [&]() static {}; // expected-error {{a static lambda cannot have any captures}}
+  auto SC2 = [&x]() static {}; // expected-error {{a static lambda cannot have any captures}}
+  auto SC3 = [=]() static {}; // expected-error {{a static lambda cannot have any captures}}
+}
Index: clang/test/Parser/cxx2b-lambdas-ext-warns.cpp
===
--- /dev/null
+++ clang/test/Parser/cxx2b-lambdas-ext-warns.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify=expected,cxx20
+// RUN: %clang_cc1 -std=c++2b %s -verify=expected,cxx2b
+
+//cxx2b-no-diagnostics
+
+auto L1 = [] constexpr {}; // cxx20-warning {{lambda without a parameter clause is a C++2b extension}}
+auto L2 = []() static {}; // cxx20-error {{static lambdas is a C++2b extension}}
+auto L3 = [] static {}; // cxx20-error {{static lambdas is a C++2b extension}} // cxx20-warning {{lambda without a parameter clause is a C++2b extension}}
Index: clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
@@ -0,0 +1,129 @@
+// RUN: %clang_cc1 -std=c++2b %s -emit-llvm -triple x86_64-linux -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++2b %s -emit-llvm -triple x86_64-windows-msvc -o - | FileCheck %s
+
+struct Functor {
+  static int operator()(int x, int y) {
+return x + y;
+  }
+};
+
+auto GetALambda() {
+  return [](int x, int y) static {
+return x + y;
+  };
+}
+
+void CallsTheLambda() {
+  GetALambda()(1, 2);
+}
+
+// CHECK:  define {{.*}}CallsTheLambda{{.*}}
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %call = call noundef i32 {{.*}}(i32 noundef 1, i32 noundef 2)
+// CHECK-NEXT:   ret void
+// CHECK-NEXT: }
+
+void call_static_call_operator() {
+  Functor f;
+  f(101, 102);
+  f.operator()(201, 202);
+  Functor{}(301, 302);
+}
+
+// CHECK:  define {{.*}}call_static_call_operator{{.*}}
+// CHECK-NEXT: entry:
+// CHECK:{{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 101, i32 noundef 102)
+// CHECK-NEXT:   {{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 201, i32 noundef 202)
+// CHECK-NEXT:   {{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 301, i32 noundef 302)
+// CH

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D111283#3783250 , @ychen wrote:

> Hi @mizvekov , I noticed that deduction for partial ordering also inherits 
> this new behavior. Do you think partial ordering could opt out of this, or is 
> it better for partial ordering to deal with the new sugared types?

I would expect partial ordering to be performed only using canonical types, 
that type sugar should make no difference there.

Note that previous to this patch, we would already produce sugar on deduction, 
but the behavior with regards to deducing the same template parameter from 
multiple places was pretty badly behaved: We would just keep the sugar from the 
first deduction, and ignore any subsequent ones. That meant that the deduction 
order had an effect on the result. With this patch, deduction becomes order 
agnostic.

What kind of difference are you seeing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

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

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:863
+  Tok.addModifier(HighlightingModifier::Declaration);
+if (const auto Func = dyn_cast(Decl)) {
+  if (Func->isThisDeclarationADefinition())

dgoldman wrote:
> Instead of doing it like this, I wonder if would make more sense to call 
> getDefinition from 
> https://cs.github.com/llvm/llvm-project/blob/ae071a59bc6cc09e0e0043e0ef1d9725853f1681/clang-tools-extra/clangd/XRefs.cpp#L76
>  and if it matches Decl, add the Definition modifier?
Won't that incur an additional look-up or some other type of work? I'm not 
deeply familiar with the implementation, but a cursory glance seems to suggests 
that isThisDeclarationADefinition() is just an accessor, while getDefinition() 
"does things".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

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

[PATCH] D133194: rewording note note_constexpr_invalid_cast

[Aaron Ballman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGea26ed1f9c37: Rewording note note_constexpr_invalid_cast 
(authored by Codesbyusman, committed by aaron.ballman).

Changed prior to commit:
  https://reviews.llvm.org/D133194?vs=459296&id=459422#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133194

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/Sema/cast.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp

Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -11,6 +11,10 @@
 
 }
 
+int array[(long)(char *)0]; // expected-warning {{variable length arrays are a C99 feature}} \
+// expected-warning {{variable length array folded to constant array as an extension}} \
+// expected-note {{cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression}}
+
 typedef decltype(sizeof(char)) size_t;
 
 template constexpr T id(const T &t) { return t; }
Index: clang/test/Sema/cast.c
===
--- clang/test/Sema/cast.c
+++ clang/test/Sema/cast.c
@@ -1,4 +1,8 @@
-// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown %s -verify
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown %s -verify -Wvla
+
+int array[(long)(char *)0]; // expected-warning {{variable length array used}} \
+// expected-warning {{variable length array folded to constant array as an extension}} \
+// expected-note {{this conversion is not allowed in a constant expression}}
 
 typedef struct { unsigned long bits[(((1) + (64) - 1) / (64))]; } cpumask_t;
 cpumask_t x;
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -8178,7 +8178,8 @@
   return LValueExprEvaluatorBaseTy::VisitCastExpr(E);
 
 case CK_LValueBitCast:
-  this->CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+  this->CCEDiag(E, diag::note_constexpr_invalid_cast)
+  << 2 << Info.Ctx.getLangOpts().CPlusPlus;
   if (!Visit(E->getSubExpr()))
 return false;
   Result.Designator.setInvalid();
@@ -8889,9 +8890,10 @@
 Result.Designator.setInvalid();
 if (SubExpr->getType()->isVoidPointerType())
   CCEDiag(E, diag::note_constexpr_invalid_cast)
-<< 3 << SubExpr->getType();
+  << 3 << SubExpr->getType();
 else
-  CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+  CCEDiag(E, diag::note_constexpr_invalid_cast)
+  << 2 << Info.Ctx.getLangOpts().CPlusPlus;
   }
 }
 if (E->getCastKind() == CK_AddressSpaceConversion && Result.IsNullPtr)
@@ -8928,7 +8930,8 @@
 return ZeroInitialization(E);
 
   case CK_IntegralToPointer: {
-CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+CCEDiag(E, diag::note_constexpr_invalid_cast)
+<< 2 << Info.Ctx.getLangOpts().CPlusPlus;
 
 APValue Value;
 if (!EvaluateIntegerOrLValue(SubExpr, Value, Info))
@@ -13587,7 +13590,8 @@
   }
 
   case CK_PointerToIntegral: {
-CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+CCEDiag(E, diag::note_constexpr_invalid_cast)
+<< 2 << Info.Ctx.getLangOpts().CPlusPlus;
 
 LValue LV;
 if (!EvaluatePointer(SubExpr, LV, Info))
Index: clang/include/clang/Basic/DiagnosticASTKinds.td
===
--- clang/include/clang/Basic/DiagnosticASTKinds.td
+++ clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -11,8 +11,9 @@
 // Constant expression diagnostics. These (and their users) belong in Sema.
 def note_expr_divide_by_zero : Note<"division by zero">;
 def note_constexpr_invalid_cast : Note<
-  "%select{reinterpret_cast|dynamic_cast|cast that performs the conversions of"
-  " a reinterpret_cast|cast from %1}0 is not allowed in a constant expression"
+  "%select{reinterpret_cast|dynamic_cast|%select{this conversion|cast that"
+  " performs the conversions of a reinterpret_cast}1|cast from %1}0"
+  " is not allowed in a constant expression"
   "%select{| in C++ standards before C++20||}0">;
 def note_constexpr_invalid_downcast : Note<
   "cannot cast object of dynamic type %0 to type %1">;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -136,6 +136,8 @@
 - no_sanitize("...") on a global variable for known but not relevant sanitizers
   is now just 

[clang] ea26ed1 - Rewording note note_constexpr_invalid_cast

[Aaron Ballman via cfe-commits]

Author: Muhammad Usman Shahid
Date: 2022-09-12T07:47:39-04:00
New Revision: ea26ed1f9c37465e0123c3357e459dd819c7d402

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

LOG: Rewording note note_constexpr_invalid_cast

The diagnostics here are correct, but the note is really silly. It
talks about reinterpret_cast in C code. So rewording it for c mode by
using another %select{}.
```
int array[(long)(char *)0];
```
previous note:
```
cast that performs the conversions of a reinterpret_cast is not allowed in a 
constant expression
```
reworded note:
```
this conversion is not allowed in a constant expression
```

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticASTKinds.td
clang/lib/AST/ExprConstant.cpp
clang/test/Sema/cast.c
clang/test/SemaCXX/constant-expression-cxx11.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4804856bc8f5c..75a57c5d18d8b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -136,6 +136,8 @@ Improvements to Clang's diagnostics
 - no_sanitize("...") on a global variable for known but not relevant sanitizers
   is now just a warning. It now says that this will be ignored instead of
   incorrectly saying no_sanitize only applies to functions and methods.
+- No longer mention ``reinterpet_cast`` in the invalid constant expression
+  diagnostic note when in C mode.
 
 Non-comprehensive list of changes in this release
 -

diff  --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index 7f47dc8ce0d88..c18b8a8fa0e60 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -11,8 +11,9 @@ let Component = "AST" in {
 // Constant expression diagnostics. These (and their users) belong in Sema.
 def note_expr_divide_by_zero : Note<"division by zero">;
 def note_constexpr_invalid_cast : Note<
-  "%select{reinterpret_cast|dynamic_cast|cast that performs the conversions of"
-  " a reinterpret_cast|cast from %1}0 is not allowed in a constant expression"
+  "%select{reinterpret_cast|dynamic_cast|%select{this conversion|cast that"
+  " performs the conversions of a reinterpret_cast}1|cast from %1}0"
+  " is not allowed in a constant expression"
   "%select{| in C++ standards before C++20||}0">;
 def note_constexpr_invalid_downcast : Note<
   "cannot cast object of dynamic type %0 to type %1">;

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 26822a64b14c6..64a64d5dfc3dd 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8178,7 +8178,8 @@ class LValueExprEvaluator
   return LValueExprEvaluatorBaseTy::VisitCastExpr(E);
 
 case CK_LValueBitCast:
-  this->CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+  this->CCEDiag(E, diag::note_constexpr_invalid_cast)
+  << 2 << Info.Ctx.getLangOpts().CPlusPlus;
   if (!Visit(E->getSubExpr()))
 return false;
   Result.Designator.setInvalid();
@@ -8889,9 +8890,10 @@ bool PointerExprEvaluator::VisitCastExpr(const CastExpr 
*E) {
 Result.Designator.setInvalid();
 if (SubExpr->getType()->isVoidPointerType())
   CCEDiag(E, diag::note_constexpr_invalid_cast)
-<< 3 << SubExpr->getType();
+  << 3 << SubExpr->getType();
 else
-  CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+  CCEDiag(E, diag::note_constexpr_invalid_cast)
+  << 2 << Info.Ctx.getLangOpts().CPlusPlus;
   }
 }
 if (E->getCastKind() == CK_AddressSpaceConversion && Result.IsNullPtr)
@@ -8928,7 +8930,8 @@ bool PointerExprEvaluator::VisitCastExpr(const CastExpr 
*E) {
 return ZeroInitialization(E);
 
   case CK_IntegralToPointer: {
-CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+CCEDiag(E, diag::note_constexpr_invalid_cast)
+<< 2 << Info.Ctx.getLangOpts().CPlusPlus;
 
 APValue Value;
 if (!EvaluateIntegerOrLValue(SubExpr, Value, Info))
@@ -13587,7 +13590,8 @@ bool IntExprEvaluator::VisitCastExpr(const CastExpr *E) 
{
   }
 
   case CK_PointerToIntegral: {
-CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+CCEDiag(E, diag::note_constexpr_invalid_cast)
+<< 2 << Info.Ctx.getLangOpts().CPlusPlus;
 
 LValue LV;
 if (!EvaluatePointer(SubExpr, LV, Info))

diff  --git a/clang/test/Sema/cast.c b/clang/test/Sema/cast.c
index 1b7519eb87ebc..37085d159d5e8 100644
--- a/clang/test/Sema/cast.c
+++ b/clang/test/Sema/cast.c
@@ -1,4 +1,8 @@
-// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unkno

[clang] f0403c8 - Fix build of Lex unit test with CLANG_DYLIB

[Jonas Hahnfeld via cfe-commits]

Author: Jonas Hahnfeld
Date: 2022-09-12T13:49:57+02:00
New Revision: f0403c853bc93fe1127fef7493a4feff1479191e

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

LOG: Fix build of Lex unit test with CLANG_DYLIB

If CLANG_LINK_CLANG_DYLIB, clang_target_link_libraries ignores all
indivial libraries and only links clang-cpp. As LLVMTestingSupport
is separate, pass it via target_link_libraries directly.

Added: 


Modified: 
clang/unittests/Lex/CMakeLists.txt

Removed: 




diff  --git a/clang/unittests/Lex/CMakeLists.txt 
b/clang/unittests/Lex/CMakeLists.txt
index 5b498f54fb0af..bed5fd9186f22 100644
--- a/clang/unittests/Lex/CMakeLists.txt
+++ b/clang/unittests/Lex/CMakeLists.txt
@@ -20,6 +20,9 @@ clang_target_link_libraries(LexTests
   clangLex
   clangParse
   clangSema
+  )
 
+target_link_libraries(LexTests
+  PRIVATE
   LLVMTestingSupport
   )



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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[dongjunduo via Phabricator via cfe-commits]

dongjunduo updated this revision to Diff 459423.
dongjunduo added a comment.

fix related nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -256,17 +256,10 @@
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
-if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-  // replace the suffix to '.json' directly
-  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-  if (llvm::sys::fs::is_directory(TracePath))
-llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-  Path.assign(TracePath);
-}
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
+assert(!TracePath.empty() && "`-ftime-trace=` is empty");
 if (auto profilerOutput = Clang->createOutputFile(
-Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+TracePath.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
   llvm::timeTraceProfilerWrite(*profilerOutput);
   profilerOutput.reset();
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,3 +1,6 @@
+// RUN: rm -rf %T/exe && mkdir %T/exe
+// RUN: %clangxx -### -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-OPT %s
 // RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
@@ -17,6 +20,7 @@
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
 // RUN:   | FileCheck %s
 
+// CHECK-OPT:  "-ftime-trace={{.*}}{{/|}}exe{{/|}}check-time-trace{{.*}}.json"
 // CHECK:  "beginningOfTime": {{[0-9]{16},}}
 // CHECK-NEXT: "traceEvents": [
 // CHECK:  "args":
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4510,6 +4510,95 @@
   llvm_unreachable("invalid phase in ConstructPhaseAction");
 }
 
+// Infer data storing path of the options `-ftime-trace`, `-ftime-trace=`
+static void inferTimeTracePath(Compilation &C) {
+  bool HasTimeTrace = C.getArgs().hasArg(options::OPT_ftime_trace);
+  bool HasTimeTraceFile = C.getArgs().hasArg(options::OPT_ftime_trace_EQ);
+  // Whether `-ftime-trace` or `-ftime-trace=` are specified
+  if (!HasTimeTrace && !HasTimeTraceFile)
+return;
+
+  // If `-ftime-trace=` is specified, TracePath is the .
+  // Else if there is a linking job, TracePath is the parent path of .exe,
+  // then the OutputFile's name may be appended to it.
+  // Else, TracePath is "",
+  // then the full OutputFile's path may be appended to it.
+  SmallString<128> TracePath("");
+  if (HasTimeTraceFile) {
+TracePath = SmallString<128>(
+C.getArgs().getLastArg(options::OPT_ftime_trace_EQ)->getValue());
+  } else {
+// Get linking executable file's parent path as TracePath's parent path,
+// default is ".". Filename may be determined and added into TracePath then.
+//
+// e.g. executable file's path: /usr/local/a.out
+//  its parent's path:  /usr/local
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass) {
+assert(!J.getOutputFilenames().empty() &&
+   "linking output filename is empty");
+auto OutputFilePath =
+SmallString<128>(J.getOutputFilenames()[0].c_str());
+if (llvm::sys::path::has_parent_path(OutputFilePath)) {
+  TracePath = llvm::sys::path::parent_path(OutputFilePath);
+} else {
+  TracePath = SmallString<128>(".");
+}
+break;
+  }
+}
+  }
+
+  // Add or replace the modified -ftime-trace=` to all clang jobs
+  for (auto &J : C.getJobs()) {
+if (J.getSource().getKind() == Action::AssembleJobClass ||
+J.getSource().getKind() == Action::BackendJobClass ||
+J.getSource().getKind() == Action::CompileJobClass) {
+  SmallString<128> TracePathReal = TracePath;
+  SmallString<128> OutputPath(J.getOutputFilenames()[0].c_str());
+  SmallString<128> arg("-ftime-trace=");
+  i

[PATCH] D133668: [HLSL] Use _BitInt(16) for int16_t to avoid promote to int.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133668#3783090 , @beanz wrote:

> HLSL deviates from C here. HLSL doesn't actually have `short` (although I'm 
> actually not sure we should disable it in Clang). We do have `int16_t`, but 
> we don't promote `int16_t` to `int`. We discussed changing codegen to disable 
> promotion for HLSL, but it seemed more straightforward to me to just define 
> `int16_t` as `_BitInt(16)`.

Okay, that's good to know! If you don't intend to *ever* conform to the 
standard in this area, then I think this approach is very reasonable. But you 
should know up front that you're constraining yourself here. (Changing the 
underlying type in the future is an ABI break: https://godbolt.org/z/P6ndrzMab, 
note the name mangling.)




Comment at: clang/lib/Basic/Targets/DirectX.h:66
 
+  bool hasBitIntType() const override { return true; }
   bool hasFeature(StringRef Feature) const override {

This change requires more testing/thought, IMO -- do you support 128-bit 
operations? When we bump that limit to be arbitrarily high, should DX have the 
arbitrary limits or do you need to enforce something lower? Have you considered 
how you want to pack these into structures or other data layout considerations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133668

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[dongjunduo via Phabricator via cfe-commits]

dongjunduo added a comment.

In D131469#3775741 , @MaskRay wrote:

> I appreciate the detailed summary. It has sufficient information but I think 
> it can be rephased to be conciser.
> I request changes for the verbosity and the possibly functionality issue.
>
>> We can use -ftime-trace or -ftime-trace= to switch on the TimeProfiler.
>
> "We can use" can be omitted => "-ftime-trace or -ftime-trace= enables 
> ..."
>
>> The source files should be compiled into object files (.o) by clang, and 
>> then linked into executable file by the linker.
>
> This is basic knowledge and can be removed.
>
>   "-ftime-trace or -ftime-trace= enables ..."
>
>
>
>> Where to store for now?
>
> If we want a detailed description, the whole paragraph is really something 
> which should go to https://clang.llvm.org/docs/UsersManual.html
> Then, this patch summary (commit message) can just refer to it.
>
> ---
>
> I'd delete everything and keep just
>
> `-ftime-trace` and `-ftime-trace=` enable the TimeProfiler.
> When Clang does compile and link actions in one command and `-ftime-trace` is 
> used, the JSON file currently goes to a temporary directory, e.g.
>
>   $ clang++ -ftime-trace -o main.out /demo/main.cpp
>   $ ls .
>   main.out
>   $ ls /tmp/
>   main-[random-string].json
>
> The JSON file location is undesired. This patch changes the location based on 
> the specified ...
>
> ---
>
> What if an input file and the output is in directories, e.g.
>
>   % myclang -ftime-trace a/a.c -o b/a; ls *.json
>   a-a7cffa.json
>
> Does the patch achieve what it intends to do?

Nice idea. I have simplified the related description. : )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D133088: [Clang] Reword diagnostic for scope identifier with linkage

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133088

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

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

[Luke Nihlen via Phabricator via cfe-commits]

luken-google added a comment.

Friendly ping on this, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133052

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

[PATCH] D133574: [C2x] reject type definitions in offsetof

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133574#3781907 , @inclyc wrote:

> Use RAII object to maintain the Parser state
>
>> have you explored making a new DeclSpecContext and modifying 
>> isDefiningTypeSpecifierContext()? I think that would likely be a cleaner 
>> approach.
>
> Emm, I've tried passing a DeclaratorContext into `ParseTypeName()`
>
>   SourceLocation TypeLoc = Tok.getLocation();
>   InBuiltInOffsetOfBaseRAIIObject InOffsetof(*this, true);
>   TypeResult Ty = ParseTypeName(nullptr, /*Context=???*/); 
>
> But defining a new DeclaratorContext I have to complete a bunch of `case`
> statements.
>
>   // Parser.h
>   static bool isTypeSpecifier(DeclSpecContext DSC);
>   static AllowDefiningTypeSpec isDefiningTypeSpecifierContext(DeclSpecContext 
> DSC, bool IsCPlusPlus);
>   static bool isOpaqueEnumDeclarationContext(DeclSpecContext DSC);
>   /* ... */
>
> And I think it is somehow strange to determine these properties within
> __builtin_offsetof()? I'm not sure if it is really appropriate to define a
> special context for a built-in function. This place should only need to
> forbidden definitions, right?

The thing is: it really is a declaration context; one in which type 
declarations are not allowed. So yes, you do have to fill out a bunch of fully 
covered switches in places, but I still think that's the correct way to model 
this instead of introducing a one-off RAII object.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133574

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

[PATCH] D133634: [clang] Allow vector of BitInt

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a subscriber: erichkeane.
aaron.ballman added a comment.

In D133634#3783160 , @python3kgae 
wrote:

> In D133634#3782999 , @xbolva00 
> wrote:
>
>> Missing IR tests. No issues in codegen?
>>
>> Very poor description. Why it was disabled? Why we can enable it now?
>
> I'm not sure why it is disabled.
> I guess it is never enabled and just assert in ASTContext::getExtVectorType 
> before.
> Then 
> https://github.com/llvm/llvm-project/commit/c9edf843fcf954132271214445857498fb47bb72
>  make it an error instead of assert.

It was disabled because it wasn't intentionally enabled -- we've not thought 
through the best way to expose vectors of _BitInt yet and we didn't want anyone 
to get tied into an accidental ABI by leaving them exposed.

> I'm enabling it for HLSL to use _BitInt(16) as 16bit int at 
> https://reviews.llvm.org/D133668
> This PR only makes sure it does not fail at AST level.
> https://reviews.llvm.org/D133668 will fix the fail in Mangling when codeGen.

I think this requires more consideration. I think we *want* to enable vectors 
of `_BitInt` types, but I think we need a more full-featured plan for 
supporting them. For example, do we want to allow a vector of `_BitInt(5)` 
objects, or do we want to support only powers-of-two sized `_BitInt`s? How do 
we ensure ABI compatibility with GCC for these vector types? That sort of thing.

CC @erichkeane who worked on the initial `_ExtInt` implementation in case he's 
got additional thoughts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133634

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

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Hi Matheus, an early heads up: this commit is causing clang crashes on some of 
our code. I'll post more details a bit later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

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

[PATCH] D133694: [Clang][OpenMP] Fix use_device_addr

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 created this revision.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
doru1004 requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

With the current implementation, the use_device_addr does not correctly use a 
pointer that was already mapped to the device and ends up in segmentation 
fault. The test showcases the situation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133694

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp


Index: clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -fopenmp-version=50 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple 
powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+int main() {
+float x_array[256];
+float *x = &x_array[0];
+
+// make x available on the GPU
+#pragma omp target data map(tofrom:x[0:256])
+{
+#pragma omp target data use_device_addr(x)
+{
+x[0] = 2;
+}
+}
+return x[0] == 2;
+}
+
+// CHECK-LABEL: @main()
+// CHECK: [[X:%.+]] = alloca ptr, align 8
+// CHECK: call void @__tgt_target_data_begin_mapper(
+// CHECK: [[LOADED_X:%.+]] = load ptr, ptr [[X]], align 8
+// CHECK: [[BASE_PTR_GEP:%.+]] = getelementptr inbounds [1 x ptr], ptr 
[[OFFLOAD_BASE_PTR:%.+]], i32 0, i32 0
+// CHECK: store ptr [[LOADED_X]], ptr [[BASE_PTR_GEP]], align 8
+// CHECK: [[OFFLOAD_PTR_GEP:%.+]] = getelementptr inbounds [1 x ptr], ptr 
[[OFFLOAD_PTR:%.+]], i32 0, i32 0
+// CHECK: store ptr [[LOADED_X]], ptr [[OFFLOAD_PTR_GEP]], align 8
+// CHECK: getelementptr inbounds [1 x ptr], ptr [[OFFLOAD_BASE_PTR]], i32 0, 
i32 0
+// CHECK: getelementptr inbounds [1 x ptr], ptr [[OFFLOAD_PTR]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_begin_mapper(
+// CHECK: [[LOADED_DEVICE_X:%.+]] = load ptr, ptr [[BASE_PTR_GEP]], align 8
+// CHECK: %arrayidx5 = getelementptr inbounds float, ptr %13, i64 0
+// CHECK: store float 2.00e+00, ptr %arrayidx5, align 4
+// CHECK: getelementptr inbounds [1 x ptr], ptr [[OFFLOAD_BASE_PTR]], i32 0, 
i32 0
+// CHECK: getelementptr inbounds [1 x ptr], ptr [[OFFLOAD_PTR]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_end_mapper(
+// CHECK: call void @__tgt_target_data_end_mapper(
+
+#endif
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -7241,6 +7241,7 @@
 // declaration used by the mapping logic. In some cases we may get
 // OMPCapturedExprDecl that refers to the original declaration.
 const ValueDecl *MatchingVD = OrigVD;
+bool isPartOfAStruct = false;
 if (const auto *OED = dyn_cast(MatchingVD)) {
   // OMPCapturedExprDecl are used to privative fields of the current
   // structure.
@@ -7248,6 +7249,7 @@
   assert(isa(ME->getBase()) &&
  "Base should be the current struct!");
   MatchingVD = ME->getMemberDecl();
+  isPartOfAStruct = true;
 }
 
 // If we don't have information about the current list item, move on to
@@ -7259,8 +7261,11 @@
 Address PrivAddr = InitAddrIt->getSecond();
 // For declrefs and variable length array need to load the pointer for
 // correct mapping, since the pointer to the data was passed to the 
runtime.
-if (isa(Ref->IgnoreParenImpCasts()) ||
-MatchingVD->getType()->isArrayType()) {
+// Pointer types are already mapped correctly so no need to do a load 
unless
+// the pointer type is part of a struct.
+if ((isa(Ref->IgnoreParenImpCasts()) ||
+MatchingVD->getType()->isArrayType()) &&
+(isPartOfAStruct || !MatchingVD->getType()->isPointerType())) {
   QualType PtrTy = getContext().getPointerType(
   OrigVD->getType().getNonReferenceType());
   PrivAddr = EmitLoadOfPointer(
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -8691,7 +8691,7 @@
   DeferredInfo[nullptr].emplace_back(IE, VD, /*ForDeviceAddr=*/true);
 } else {
   llvm::Value *Ptr;
-  if (IE->isGLValue())
+  if (IE->isGLValue() && !IE->getType()->isPointerType())
 Ptr = CGF.EmitLValue(IE).getPointer(CGF);
   else
 Ptr = CGF.EmitScalarExpr(IE);


Index: clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp
===

[PATCH] D131688: [clang][dataflow][NFC] Remove LatticeJoinEffect from framework interfaces

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D131688#3716467 , @xazax.hun wrote:

> Returning whether the lattice was changed can be a nice optimization when 
> comparing lattice elements is expensive. On the other hand, it can be one 
> more source of errors. I am fine with either approach, both can make sense 
> depending on whether the goal is to make the interface easier to use or to 
> have superb performance.

Agreed and the performance consideration was the original motivation. But, then 
we never used it! Indeed, we'd have to rework the worklist algorithm to make 
use of return value so that, for each block, it saved the state immediately 
before the block (that is, after the join from incoming branches). Currently, 
we only save the state *after* the block.

That's why I suggested to Eric that we should remove it. We can revisit in the 
future: if it seems a valuable source of performance improvement, we can 
consider how best to modify the system at that point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131688

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

[PATCH] D133698: [clang][dataflow] SignAnalysis, edgeTransfer, branchTransfer

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: xazax.hun, gribozavr2, sgatev, ymandel, samestep.
Herald added subscribers: steakhal, gamesh411, Szelethus, dkrupp, rnkovacs, 
mgorny.
Herald added a reviewer: Szelethus.
Herald added a reviewer: NoQ.
Herald added a project: All.
martong requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch is an auxiliary material for an RFC on discourse.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133698

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -344,6 +344,7 @@
 llvm::any_cast(&State.Lattice.Value);
 auto [_, InsertSuccess] =
 AnnotationStates.insert({It->second, StateT{*Lattice, State.Env}});
+(void)_;
 (void)InsertSuccess;
 assert(InsertSuccess);
   };
Index: clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
@@ -0,0 +1,590 @@
+//===- unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+//  This file defines a simplistic version of Sign Analysis as an example
+//  of a forward, monotonic dataflow analysis. The analysis tracks all
+//  variables in the scope, but lacks escape analysis.
+//
+//===--===//
+
+#include "TestingSupport.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace dataflow {
+namespace {
+using namespace ast_matchers;
+
+// Models the signedness of a variable, for all paths through
+// the program.
+struct SignLattice {
+  enum class SignState : int {
+Bottom,
+Negative,
+Zero,
+Positive,
+Top,
+  };
+  SignState State;
+
+  constexpr SignLattice() : State(SignState::Bottom) {}
+  constexpr SignLattice(int64_t V)
+  : State(V == 0 ? SignState::Zero
+ : (V < 0 ? SignState::Negative : SignState::Positive)) {}
+  constexpr SignLattice(SignState S) : State(S) {}
+
+  static constexpr SignLattice bottom() {
+return SignLattice(SignState::Bottom);
+  }
+  static constexpr SignLattice negative() {
+return SignLattice(SignState::Negative);
+  }
+  static constexpr SignLattice zero() { return SignLattice(SignState::Zero); }
+  static constexpr SignLattice positive() {
+return SignLattice(SignState::Positive);
+  }
+  static constexpr SignLattice top() { return SignLattice(SignState::Top); }
+
+  friend bool operator==(const SignLattice &Lhs, const SignLattice &Rhs) {
+return Lhs.State == Rhs.State;
+  }
+  friend bool operator!=(const SignLattice &Lhs, const SignLattice &Rhs) {
+return !(Lhs == Rhs);
+  }
+
+  LatticeJoinEffect join(const SignLattice &Other) {
+if (*this == Other || Other == bottom() || *this == top())
+  return LatticeJoinEffect::Unchanged;
+
+if (*this == bottom()) {
+  *this = Other;
+  return LatticeJoinEffect::Changed;
+}
+
+*this = top();
+return LatticeJoinEffect::Changed;
+  }
+};
+
+std::ostream &operator<<(std::ostream &OS, const SignLattice &L) {
+  switch (L.State) {
+  case SignLattice::SignState::Bottom:
+return OS << "Bottom";
+  case SignLattice::SignState::Negative:
+return OS << "Negative";

[PATCH] D133694: [Clang][OpenMP] Fix use_device_addr

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 updated this revision to Diff 459452.

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

https://reviews.llvm.org/D133694

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp


Index: clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -fopenmp-version=50 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple 
powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+int main() {
+float x_array[256];
+float *x = &x_array[0];
+
+// make x available on the GPU
+#pragma omp target data map(tofrom:x[0:256])
+{
+#pragma omp target data use_device_addr(x)
+{
+x[0] = 2;
+}
+}
+return x[0] == 2;
+}
+
+// CHECK-LABEL: @main()
+// CHECK: [[X:%.+]] = alloca ptr, align 8
+// CHECK: call void @__tgt_target_data_begin_mapper(
+// CHECK: [[LOADED_X:%.+]] = load ptr, ptr [[X]], align 8
+// CHECK: [[BASE_PTR_GEP:%.+]] = getelementptr inbounds [1 x ptr], ptr 
[[OFFLOAD_BASE_PTR:%.+]], i32 0, i32 0
+// CHECK: store ptr [[LOADED_X]], ptr [[BASE_PTR_GEP]], align 8
+// CHECK: [[OFFLOAD_PTR_GEP:%.+]] = getelementptr inbounds [1 x ptr], ptr 
[[OFFLOAD_PTR:%.+]], i32 0, i32 0
+// CHECK: store ptr [[LOADED_X]], ptr [[OFFLOAD_PTR_GEP]], align 8
+// CHECK: getelementptr inbounds [1 x ptr], ptr [[OFFLOAD_BASE_PTR]], i32 0, 
i32 0
+// CHECK: getelementptr inbounds [1 x ptr], ptr [[OFFLOAD_PTR]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_begin_mapper(
+// CHECK: [[LOADED_DEVICE_X:%.+]] = load ptr, ptr [[BASE_PTR_GEP]], align 8
+// CHECK: %arrayidx5 = getelementptr inbounds float, ptr %13, i64 0
+// CHECK: store float 2.00e+00, ptr %arrayidx5, align 4
+// CHECK: getelementptr inbounds [1 x ptr], ptr [[OFFLOAD_BASE_PTR]], i32 0, 
i32 0
+// CHECK: getelementptr inbounds [1 x ptr], ptr [[OFFLOAD_PTR]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_end_mapper(
+// CHECK: call void @__tgt_target_data_end_mapper(
+
+#endif
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -7241,6 +7241,7 @@
 // declaration used by the mapping logic. In some cases we may get
 // OMPCapturedExprDecl that refers to the original declaration.
 const ValueDecl *MatchingVD = OrigVD;
+bool isPartOfAStruct = false;
 if (const auto *OED = dyn_cast(MatchingVD)) {
   // OMPCapturedExprDecl are used to privative fields of the current
   // structure.
@@ -7248,6 +7249,7 @@
   assert(isa(ME->getBase()) &&
  "Base should be the current struct!");
   MatchingVD = ME->getMemberDecl();
+  isPartOfAStruct = true;
 }
 
 // If we don't have information about the current list item, move on to
@@ -7259,8 +7261,11 @@
 Address PrivAddr = InitAddrIt->getSecond();
 // For declrefs and variable length array need to load the pointer for
 // correct mapping, since the pointer to the data was passed to the 
runtime.
-if (isa(Ref->IgnoreParenImpCasts()) ||
-MatchingVD->getType()->isArrayType()) {
+// Pointer types are already mapped correctly so no need to do a load 
unless
+// the pointer type is part of a struct.
+if ((isa(Ref->IgnoreParenImpCasts()) ||
+MatchingVD->getType()->isArrayType()) &&
+(isPartOfAStruct || !MatchingVD->getType()->isPointerType())) {
   QualType PtrTy = getContext().getPointerType(
   OrigVD->getType().getNonReferenceType());
   PrivAddr = EmitLoadOfPointer(
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -8691,7 +8691,7 @@
   DeferredInfo[nullptr].emplace_back(IE, VD, /*ForDeviceAddr=*/true);
 } else {
   llvm::Value *Ptr;
-  if (IE->isGLValue())
+  if (IE->isGLValue() && !IE->getType()->isPointerType())
 Ptr = CGF.EmitLValue(IE).getPointer(CGF);
   else
 Ptr = CGF.EmitScalarExpr(IE);


Index: clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+int main() {
+float x_array[2

[PATCH] D133298: [clang-rename] RecursiveSymbolVisitor - visit namespace aliases and using directives

[imctrading via Phabricator via cfe-commits]

imctrading added a comment.

ping


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

https://reviews.llvm.org/D133298

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

[clang] 1d51bb8 - [Clang] Reword diagnostic for scope identifier with linkage

[Jun Zhang via cfe-commits]

Author: Jun Zhang
Date: 2022-09-12T22:40:54+08:00
New Revision: 1d51bb824f25140a5b1aa783f6860ac3e7f7d16d

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

LOG: [Clang] Reword diagnostic for scope identifier with linkage

If the declaration of an identifier has block scope, and the identifier has
external or internal linkage, the declaration shall have no initializer for
the identifier.

Clang now gives a more suitable diagnosis for this case.
Fixes https://github.com/llvm/llvm-project/issues/57478

Signed-off-by: Jun Zhang 

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

Added: 
clang/test/Sema/err-decl-block-extern-no-init.c

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDecl.cpp
clang/test/Parser/cxx1z-decomposition.cpp
clang/test/Sema/array-init.c
clang/test/Sema/private-extern.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 75a57c5d18d8b..30402411178d4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -138,6 +138,9 @@ Improvements to Clang's diagnostics
   incorrectly saying no_sanitize only applies to functions and methods.
 - No longer mention ``reinterpet_cast`` in the invalid constant expression
   diagnostic note when in C mode.
+- Clang will now give a more suitale diagnostic for declaration of block
+  scope identifiers that have external/internal linkage that has an 
initializer.
+  Fixes `Issue 57478: `_.
 
 Non-comprehensive list of changes in this release
 -

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 76e18c9deff46..016affb1b3236 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5901,7 +5901,7 @@ def err_loader_uninitialized_extern_decl
 : Error<"variable %0 cannot be declared both 'extern' and with the "
 "'loader_uninitialized' attribute">;
 def err_block_extern_cant_init : Error<
-  "'extern' variable cannot have an initializer">;
+  "declaration of block scope identifier with linkage cannot have an 
initializer">;
 def warn_extern_init : Warning<"'extern' variable has an initializer">,
   InGroup>;
 def err_variable_object_no_init : Error<

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 2ad6edc121ae9..6e94da4a115eb 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -12773,8 +12773,11 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr 
*Init, bool DirectInit) {
 return;
   }
 
+  // C99 6.7.8p5. If the declaration of an identifier has block scope, and
+  // the identifier has external or internal linkage, the declaration shall
+  // have no initializer for the identifier.
+  // C++14 [dcl.init]p5 is the same restriction for C++.
   if (VDecl->isLocalVarDecl() && VDecl->hasExternalStorage()) {
-// C99 6.7.8p5. C++ has no such restriction, but that is a defect.
 Diag(VDecl->getLocation(), diag::err_block_extern_cant_init);
 VDecl->setInvalidDecl();
 return;

diff  --git a/clang/test/Parser/cxx1z-decomposition.cpp 
b/clang/test/Parser/cxx1z-decomposition.cpp
index 7abf1f9cdac56..10ef464bda50c 100644
--- a/clang/test/Parser/cxx1z-decomposition.cpp
+++ b/clang/test/Parser/cxx1z-decomposition.cpp
@@ -69,7 +69,7 @@ namespace BadSpecifiers {
 // storage-class-specifiers
 static auto &[a] = n; // expected-warning {{declared 'static' is a C++20 
extension}}
 thread_local auto &[b] = n; // expected-warning {{declared 'thread_local' 
is a C++20 extension}}
-extern auto &[c] = n; // expected-error {{cannot be declared 'extern'}} 
expected-error {{cannot have an initializer}}
+extern auto &[c] = n; // expected-error {{cannot be declared 'extern'}} 
expected-error {{declaration of block scope identifier with linkage cannot have 
an initializer}}
 struct S {
   mutable auto &[d] = n; // expected-error {{not permitted in this 
context}}
 

diff  --git a/clang/test/Sema/array-init.c b/clang/test/Sema/array-init.c
index ae3ce73ccc7a3..fcc3c13bc91da 100644
--- a/clang/test/Sema/array-init.c
+++ b/clang/test/Sema/array-init.c
@@ -48,7 +48,7 @@ void func(void) {
 
   struct threeElements *p = 7; // expected-error{{incompatible integer to 
pointer conversion initializing 'struct threeElements *' with an expression of 
type 'int'}}
 
-  extern int blockScopeExtern[3] = { 1, 3, 5 }; // expected-error{{'extern' 
variable cannot have an initializer}}
+  extern int blockScopeExtern[3] = { 1, 3, 5 }; // expected-error{{declaration 
of block scope identifier

[PATCH] D133574: [C2x] reject type definitions in offsetof

[YingChi Long via Phabricator via cfe-commits]

inclyc updated this revision to Diff 459458.
inclyc added a comment.

Use declaration context


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133574

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Parser/declarators.c
  clang/test/Sema/offsetof.c

Index: clang/test/Sema/offsetof.c
===
--- clang/test/Sema/offsetof.c
+++ clang/test/Sema/offsetof.c
@@ -70,3 +70,16 @@
   return __builtin_offsetof(Array, array[*(int*)0]); // expected-warning{{indirection of non-volatile null pointer}} expected-note{{__builtin_trap}}
 }
 
+// Reject definitions in __builtin_offsetof
+// https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm
+int test_definition(void) {
+  return __builtin_offsetof(struct A // expected-error{{'struct A' cannot be defined in '__builtin_offsetof'}} 
+  { 
+int a;
+struct B // no-error, struct B is not defined within __builtin_offsetof directly
+{
+  int c;
+  int d;
+} x;
+  }, a);
+}
Index: clang/test/Parser/declarators.c
===
--- clang/test/Parser/declarators.c
+++ clang/test/Parser/declarators.c
@@ -80,10 +80,6 @@
 struct test10 { int a; } static test10x;
 struct test11 { int a; } const test11x;
 
-// PR6216
-void test12(void) {
-  (void)__builtin_offsetof(struct { char c; int i; }, i);
-}
 
 // rdar://7608537
 struct test13 { int a; } (test13x);
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -3584,6 +3584,7 @@
   [[fallthrough]];
 case DeclaratorContext::TypeName:
 case DeclaratorContext::Association:
+case DeclaratorContext::OffsetOf:
   Error = 15; // Generic
   break;
 case DeclaratorContext::File:
@@ -3695,6 +3696,7 @@
 case DeclaratorContext::TemplateArg:
 case DeclaratorContext::TemplateTypeArg:
 case DeclaratorContext::Association:
+case DeclaratorContext::OffsetOf:
   DiagID = diag::err_type_defined_in_type_specifier;
   break;
 case DeclaratorContext::Prototype:
@@ -4784,6 +4786,7 @@
 case DeclaratorContext::FunctionalCast:
 case DeclaratorContext::RequiresExpr:
 case DeclaratorContext::Association:
+case DeclaratorContext::OffsetOf:
   // Don't infer in these contexts.
   break;
 }
@@ -5836,6 +5839,7 @@
 case DeclaratorContext::TemplateArg:
 case DeclaratorContext::TemplateTypeArg:
 case DeclaratorContext::Association:
+case DeclaratorContext::OffsetOf:
   // FIXME: We may want to allow parameter packs in block-literal contexts
   // in the future.
   S.Diag(D.getEllipsisLoc(),
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16254,7 +16254,7 @@
  SourceLocation ScopedEnumKWLoc,
  bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
  bool IsTypeSpecifier, bool IsTemplateParamOrArg,
- SkipBodyInfo *SkipBody) {
+ SkipBodyInfo *SkipBody, bool IsWithinBuiltinOffsetof) {
   // If this is not a definition, it must have a name.
   IdentifierInfo *OrigName = Name;
   assert((Name != nullptr || TUK == TUK_Definition) &&
@@ -17027,6 +17027,12 @@
cast_or_null(PrevDecl));
   }
 
+  if (IsWithinBuiltinOffsetof && TUK == TUK_Definition) {
+Diag(New->getLocation(), diag::err_type_defined_in_offsetof)
+<< Context.getTagDeclType(New);
+Invalid = true;
+  }
+
   // C++11 [dcl.type]p3:
   //   A type-specifier-seq shall not define a class or enumeration [...].
   if (getLangOpts().CPlusPlus && (IsTypeSpecifier || IsTemplateParamOrArg) &&
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -2579,7 +2579,8 @@
   }
   case tok::kw___builtin_offsetof: {
 SourceLocation TypeLoc = Tok.getLocation();
-TypeResult Ty = ParseTypeName();
+TypeResult Ty =
+ParseTypeName(/*Range*/ nullptr, DeclaratorContext::OffsetOf);
 if (Ty.isInvalid()) {
   SkipUntil(tok::r_paren, StopAtSemi);
   return ExprError();
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -2030,7 +2030,7 @@

[PATCH] D133354: [Clang]: Diagnose deprecated copy operations also in MSVC compatibility mode

[Hans Wennborg via Phabricator via cfe-commits]

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

lgtm

Maybe give Richard another day or two in case he wants to take a look, but 
otherwise landing this and seeing if there are any problems we didn't know 
about sounds good to me.


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

https://reviews.llvm.org/D133354

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

[PATCH] D133611: [clang] sort additional module maps when serializing

[Richard Howell via Phabricator via cfe-commits]

rmaz updated this revision to Diff 459467.
rmaz added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133611

Files:
  clang/lib/Serialization/ASTWriter.cpp


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1282,7 +1282,11 @@
 if (auto *AdditionalModMaps =
 Map.getAdditionalModuleMapFiles(WritingModule)) {
   Record.push_back(AdditionalModMaps->size());
-  for (const FileEntry *F : *AdditionalModMaps)
+  SmallVector ModMaps(AdditionalModMaps->begin(), 
AdditionalModMaps->end());
+  llvm::sort(ModMaps, [](const FileEntry *A, const FileEntry *B) {
+return A->getName() < B->getName();
+  });
+  for (const FileEntry *F : ModMaps)
 AddPath(F->getName(), Record);
 } else {
   Record.push_back(0);


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1282,7 +1282,11 @@
 if (auto *AdditionalModMaps =
 Map.getAdditionalModuleMapFiles(WritingModule)) {
   Record.push_back(AdditionalModMaps->size());
-  for (const FileEntry *F : *AdditionalModMaps)
+  SmallVector ModMaps(AdditionalModMaps->begin(), AdditionalModMaps->end());
+  llvm::sort(ModMaps, [](const FileEntry *A, const FileEntry *B) {
+return A->getName() < B->getName();
+  });
+  for (const FileEntry *F : ModMaps)
 AddPath(F->getName(), Record);
 } else {
   Record.push_back(0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Martin Sebor via Phabricator via cfe-commits]

msebor added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:2648
+
+Control which arrays are considered as flexible arrays members. 
+can be 1 (array of size 0, 1 and undefined are considered) or 2 (array of size 0

aaron.ballman wrote:
> This phrasing is problematic because it uses the term "flexible array 
> members" which is a term of art in the standard when it's really talking 
> about something else. We don't want `int trailing[1];` to be considered a 
> flexible array member in terms of the language rules.
A pedantic reading of the text would suggest that the type of `s.trailing` is 
incomplete at level 1 and `sizeof s.trailing` is invalid (requiring an error).  
Another minor issue is that the text refers to a size of the array when it 
actually means the number of elements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126864

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

[PATCH] D133703: [Clang] NFC: Make UnqualifiedId::Kind private for consistency.

[Corentin Jabot via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133703

Files:
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13009,7 +13009,7 @@
 Previous.clear();
   }
 
-  assert(Name.Kind == UnqualifiedIdKind::IK_Identifier &&
+  assert(Name.getKind() == UnqualifiedIdKind::IK_Identifier &&
  "name in alias declaration must be an identifier");
   TypeAliasDecl *NewTD = TypeAliasDecl::Create(Context, CurContext, UsingLoc,
Name.StartLocation,
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6586,8 +6586,8 @@
 Diag(D.getDeclSpec().getConstexprSpecLoc(), diag::err_invalid_constexpr)
 << 1 << static_cast(D.getDeclSpec().getConstexprSpecifier());
 
-  if (D.getName().Kind != UnqualifiedIdKind::IK_Identifier) {
-if (D.getName().Kind == UnqualifiedIdKind::IK_DeductionGuideName)
+  if (D.getName().getKind() != UnqualifiedIdKind::IK_Identifier) {
+if (D.getName().getKind() == UnqualifiedIdKind::IK_DeductionGuideName)
   Diag(D.getName().StartLocation,
diag::err_deduction_guide_invalid_specifier)
   << "typedef";
Index: clang/lib/Sema/DeclSpec.cpp
===
--- clang/lib/Sema/DeclSpec.cpp
+++ clang/lib/Sema/DeclSpec.cpp
@@ -413,7 +413,7 @@
 bool Declarator::isStaticMember() {
   assert(getContext() == DeclaratorContext::Member);
   return getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_static ||
- (getName().Kind == UnqualifiedIdKind::IK_OperatorFunctionId &&
+ (getName().getKind() == UnqualifiedIdKind::IK_OperatorFunctionId &&
   CXXMethodDecl::isStaticOverloadedOperator(
   getName().OperatorFunctionId.Operator));
 }
Index: clang/include/clang/Sema/DeclSpec.h
===
--- clang/include/clang/Sema/DeclSpec.h
+++ clang/include/clang/Sema/DeclSpec.h
@@ -964,10 +964,10 @@
   UnqualifiedId(const UnqualifiedId &Other) = delete;
   const UnqualifiedId &operator=(const UnqualifiedId &) = delete;
 
-public:
   /// Describes the kind of unqualified-id parsed.
   UnqualifiedIdKind Kind;
 
+public:
   struct OFI {
 /// The kind of overloaded operator.
 OverloadedOperatorKind Operator;


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13009,7 +13009,7 @@
 Previous.clear();
   }
 
-  assert(Name.Kind == UnqualifiedIdKind::IK_Identifier &&
+  assert(Name.getKind() == UnqualifiedIdKind::IK_Identifier &&
  "name in alias declaration must be an identifier");
   TypeAliasDecl *NewTD = TypeAliasDecl::Create(Context, CurContext, UsingLoc,
Name.StartLocation,
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6586,8 +6586,8 @@
 Diag(D.getDeclSpec().getConstexprSpecLoc(), diag::err_invalid_constexpr)
 << 1 << static_cast(D.getDeclSpec().getConstexprSpecifier());
 
-  if (D.getName().Kind != UnqualifiedIdKind::IK_Identifier) {
-if (D.getName().Kind == UnqualifiedIdKind::IK_DeductionGuideName)
+  if (D.getName().getKind() != UnqualifiedIdKind::IK_Identifier) {
+if (D.getName().getKind() == UnqualifiedIdKind::IK_DeductionGuideName)
   Diag(D.getName().StartLocation,
diag::err_deduction_guide_invalid_specifier)
   << "typedef";
Index: clang/lib/Sema/DeclSpec.cpp
===
--- clang/lib/Sema/DeclSpec.cpp
+++ clang/lib/Sema/DeclSpec.cpp
@@ -413,7 +413,7 @@
 bool Declarator::isStaticMember() {
   assert(getContext() == DeclaratorContext::Member);
   return getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_static ||
- (getName().Kind == UnqualifiedIdKind::IK_OperatorFunctionId &&
+ (getName().getKind() == UnqualifiedIdKind::IK_OperatorFunctionId &&
   CXXMethodDecl::isStaticOverloadedOperator(
   getName().OperatorFunctionId.Operator));
 }
Index: clang/include/clang/Sema/DeclSpec.h
===
--- clang/include/clang/Sema/DeclSpec.h
+++ clang/include/clang/Sema/DeclSpec.h
@@ -964,10 +964,10 @@
   UnqualifiedId(const Unquali

[PATCH] D133694: [Clang][OpenMP] Fix use_device_addr

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 updated this revision to Diff 459475.

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

https://reviews.llvm.org/D133694

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp


Index: clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -fopenmp-version=50 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple 
powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+int main() {
+float x_array[256];
+float *x = &x_array[0];
+
+// make x available on the GPU
+#pragma omp target data map(tofrom:x[0:256])
+{
+#pragma omp target data use_device_addr(x)
+{
+x[0] = 2;
+}
+}
+return x[0] == 2;
+}
+
+// CHECK-LABEL: @main()
+// CHECK: [[X:%.+]] = alloca ptr, align 8
+// CHECK: call void @__tgt_target_data_begin_mapper(
+// CHECK: [[LOADED_X:%.+]] = load ptr, ptr [[X]], align 8
+// CHECK: [[BASE_PTR_GEP:%.+]] = getelementptr inbounds [1 x ptr], ptr 
[[OFFLOAD_BASE_PTR:%.+]], i32 0, i32 0
+// CHECK: store ptr [[LOADED_X]], ptr [[BASE_PTR_GEP]], align 8
+// CHECK: [[OFFLOAD_PTR_GEP:%.+]] = getelementptr inbounds [1 x ptr], ptr 
[[OFFLOAD_PTR:%.+]], i32 0, i32 0
+// CHECK: store ptr [[LOADED_X]], ptr [[OFFLOAD_PTR_GEP]], align 8
+// CHECK: getelementptr inbounds [1 x ptr], ptr [[OFFLOAD_BASE_PTR]], i32 0, 
i32 0
+// CHECK: getelementptr inbounds [1 x ptr], ptr [[OFFLOAD_PTR]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_begin_mapper(
+// CHECK: [[LOADED_DEVICE_X:%.+]] = load ptr, ptr [[BASE_PTR_GEP]], align 8
+// CHECK: %arrayidx5 = getelementptr inbounds float, ptr %13, i64 0
+// CHECK: store float 2.00e+00, ptr %arrayidx5, align 4
+// CHECK: getelementptr inbounds [1 x ptr], ptr [[OFFLOAD_BASE_PTR]], i32 0, 
i32 0
+// CHECK: getelementptr inbounds [1 x ptr], ptr [[OFFLOAD_PTR]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_end_mapper(
+// CHECK: call void @__tgt_target_data_end_mapper(
+
+#endif
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -7241,6 +7241,7 @@
 // declaration used by the mapping logic. In some cases we may get
 // OMPCapturedExprDecl that refers to the original declaration.
 const ValueDecl *MatchingVD = OrigVD;
+bool isPartOfAStruct = false;
 if (const auto *OED = dyn_cast(MatchingVD)) {
   // OMPCapturedExprDecl are used to privative fields of the current
   // structure.
@@ -7248,6 +7249,7 @@
   assert(isa(ME->getBase()) &&
  "Base should be the current struct!");
   MatchingVD = ME->getMemberDecl();
+  isPartOfAStruct = true;
 }
 
 // If we don't have information about the current list item, move on to
@@ -7259,8 +7261,11 @@
 Address PrivAddr = InitAddrIt->getSecond();
 // For declrefs and variable length array need to load the pointer for
 // correct mapping, since the pointer to the data was passed to the 
runtime.
-if (isa(Ref->IgnoreParenImpCasts()) ||
-MatchingVD->getType()->isArrayType()) {
+// Pointer types are already mapped correctly so no need to do a load 
unless
+// the pointer type is part of a struct.
+if ((isa(Ref->IgnoreParenImpCasts()) ||
+ MatchingVD->getType()->isArrayType()) &&
+(isPartOfAStruct || !MatchingVD->getType()->isPointerType())) {
   QualType PtrTy = getContext().getPointerType(
   OrigVD->getType().getNonReferenceType());
   PrivAddr = EmitLoadOfPointer(
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -8691,7 +8691,7 @@
   DeferredInfo[nullptr].emplace_back(IE, VD, /*ForDeviceAddr=*/true);
 } else {
   llvm::Value *Ptr;
-  if (IE->isGLValue())
+  if (IE->isGLValue() && !IE->getType()->isPointerType())
 Ptr = CGF.EmitLValue(IE).getPointer(CGF);
   else
 Ptr = CGF.EmitScalarExpr(IE);


Index: clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+int main() {
+float x_array[

[PATCH] D133668: [HLSL] Use _BitInt(16) for int16_t to avoid promote to int.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

In D133668#3783489 , @aaron.ballman 
wrote:

> Okay, that's good to know! If you don't intend to *ever* conform to the 
> standard in this area, then I think this approach is very reasonable. But you 
> should know up front that you're constraining yourself here. (Changing the 
> underlying type in the future is an ABI break: 
> https://godbolt.org/z/P6ndrzMab, note the name mangling.)

We have the benefit of ABI escape hatches. HLSL itself doesn't define a 
permanently stable ABI since GPU hardware and runtime ABIs change too 
frequently. We instead revision our ABI every few years as the DirectX and 
Vulkan specifications evolve.

My hope is that as the HLSL language and our runtime ABIs evolve we'll be more 
and more conformant to the C standard, but there are some odd areas that we 
might never quite get there on.

The 16-bit integer math is an interesting case. Because GPUs are inherently 
SIMD machines, on many architectures you can handle twice as many 16-bit 
operations per instruction as 32-bit (yay vectors!). Combine that with HLSL's 
SPMD programming model and all scalar math is actually vector math. This makes 
integer promotion for 16-bit types severely limiting. As a result I don't 
suspect we'll ever want to conform to C here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133668

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

[PATCH] D133705: [HIP] Fix unbundling archive

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added a project: All.
yaxunl requested review of this revision.

HIP is able to unbundle archive of bundled bitcode.
However currently there are two bugs:

1. archives passed  by -l: are not unbundled.
2. archives passed as input files are not unbundled

Archives passed by -l: should not be prefixed with
prefix `lib` and appended with '.a', but still need to be prefixed with
paths in -L options.

Archives passed as input files should not be prefixed
or appended with anything.


https://reviews.llvm.org/D133705

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGenCUDA/bool-range.cu


Index: clang/test/CodeGenCUDA/bool-range.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/bool-range.cu
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -emit-llvm %s -O3 -o - -fcuda-is-device \
+// RUN:   -triple nvptx64-unknown-unknown | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+// Make sure bool loaded from memory is truncated and
+// range metadata is not emitted.
+
+// CHECK:  %0 = load i8, i8* %x
+// CHECK:  %1 = and i8 %0, 1
+// CHECK:  store i8 %1, i8* %y
+// CHECK-NOT: !range
+__global__ void test1(bool *x, bool *y) {
+  *y = *x != false;
+}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -1791,7 +1791,8 @@
   if (EmitScalarRangeCheck(Load, Ty, Loc)) {
 // In order to prevent the optimizer from throwing away the check, don't
 // attach range metadata to the load.
-  } else if (CGM.getCodeGenOpts().OptimizationLevel > 0)
+  } else if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
+ !(getLangOpts().CUDA && getLangOpts().CUDAIsDevice))
 if (llvm::MDNode *RangeInfo = getRangeForLoadFromType(Ty))
   Load->setMetadata(llvm::LLVMContext::MD_range, RangeInfo);
 


Index: clang/test/CodeGenCUDA/bool-range.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/bool-range.cu
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -emit-llvm %s -O3 -o - -fcuda-is-device \
+// RUN:   -triple nvptx64-unknown-unknown | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+// Make sure bool loaded from memory is truncated and
+// range metadata is not emitted.
+
+// CHECK:  %0 = load i8, i8* %x
+// CHECK:  %1 = and i8 %0, 1
+// CHECK:  store i8 %1, i8* %y
+// CHECK-NOT: !range
+__global__ void test1(bool *x, bool *y) {
+  *y = *x != false;
+}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -1791,7 +1791,8 @@
   if (EmitScalarRangeCheck(Load, Ty, Loc)) {
 // In order to prevent the optimizer from throwing away the check, don't
 // attach range metadata to the load.
-  } else if (CGM.getCodeGenOpts().OptimizationLevel > 0)
+  } else if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
+ !(getLangOpts().CUDA && getLangOpts().CUDAIsDevice))
 if (llvm::MDNode *RangeInfo = getRangeForLoadFromType(Ty))
   Load->setMetadata(llvm::LLVMContext::MD_range, RangeInfo);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133705: [HIP] Fix unbundling archive

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 459482.
yaxunl added a comment.
Herald added subscribers: sstefan1, MaskRay.
Herald added a reviewer: jdoerfert.

sorry. update with the correct patch.


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

https://reviews.llvm.org/D133705

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/hip-link-bundle-archive.hip

Index: clang/test/Driver/hip-link-bundle-archive.hip
===
--- clang/test/Driver/hip-link-bundle-archive.hip
+++ clang/test/Driver/hip-link-bundle-archive.hip
@@ -7,7 +7,17 @@
 // RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
 // RUN:   --target=x86_64-unknown-linux-gnu \
 // RUN:   -nogpulib %s -fgpu-rdc -L%T -lhipBundled \
-// RUN:   2>&1 | FileCheck -check-prefix=GNU %s
+// RUN:   2>&1 | FileCheck -check-prefixes=GNU,GNU-L %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-unknown-linux-gnu \
+// RUN:   -nogpulib %s -fgpu-rdc -L%T -l:libhipBundled.a \
+// RUN:   2>&1 | FileCheck -check-prefixes=GNU,GNU-LA %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-unknown-linux-gnu \
+// RUN:   -nogpulib %s -fgpu-rdc %T/libhipBundled.a \
+// RUN:   2>&1 | FileCheck -check-prefixes=GNU,GNU-A %s
 
 // RUN: touch %T/hipBundled2.lib
 // RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
@@ -15,14 +25,26 @@
 // RUN:   -nogpulib %s -fgpu-rdc -L%T -lhipBundled2 \
 // RUN:   2>&1 | FileCheck -check-prefix=MSVC %s
 
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-pc-windows-msvc \
+// RUN:   -nogpulib %s -fgpu-rdc -L%T -l:hipBundled2.lib \
+// RUN:   2>&1 | FileCheck -check-prefix=MSVC %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-pc-windows-msvc \
+// RUN:   -nogpulib %s -fgpu-rdc %T/hipBundled2.lib \
+// RUN:   2>&1 | FileCheck -check-prefix=MSVC %s
+
 // GNU: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libhipBundled.a" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
 // GNU: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx1030" {{.*}} "[[A1030]]"
 // GNU: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libhipBundled.a" "-targets=hip-amdgcn-amd-amdhsa-gfx906" "-output=[[A906:.*\.a]]" "-allow-missing-bundles"
 // GNU: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx906" {{.*}} "[[A906]]"
-// GNU: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-lhipBundled"
+// GNU-L: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-lhipBundled"
+// GNU-LA: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-l:libhipBundled.a"
+// GNU-A: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" "{{.*}}libhipBundled.a"
 
 // MSVC: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}hipBundled2.lib" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
 // MSVC: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx1030" {{.*}} "[[A1030]]"
 // MSVC: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}hipBundled2.lib" "-targets=hip-amdgcn-amd-amdhsa-gfx906" "-output=[[A906:.*\.a]]" "-allow-missing-bundles"
 // MSVC: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx906" {{.*}} "[[A906]]"
-// MSVC: "{{.*}}link{{.*}}" {{.*}}"-out:a.exe" {{.*}}"hipBundled2.lib"
+// MSVC: "{{.*}}link{{.*}}" {{.*}}"-out:a.exe" {{.*}}hipBundled2.lib"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1924,83 +1924,102 @@
   bool FoundAOB = false;
   SmallVector AOBFileNames;
   std::string ArchiveOfBundles;
-  for (auto LPath : LibraryPaths) {
-ArchiveOfBundles.clear();
-
-llvm::Triple Triple(D.getTargetTriple());
-bool IsMSVC = Triple.isWindowsMSVCEnvironment();
-for (auto Prefix : {"/libdevice/", "/"}) {
-  if (IsMSVC)
-AOBFileNames.push_back(Twine(LPath + Prefix + Lib + ".lib").str());
-  AOBFileNames.push_back(Twine(LPath + Prefix + "lib" + Lib + ".a").str());
+
+  llvm::Triple Triple(D.getTargetTriple());
+  bool IsMSVC = Triple.isWindowsMSVCEnvironment();
+  auto Ext = IsMSVC ? ".lib" : ".a";
+  if (Lib.endswith(Ext) && !Lib.startswith(":")) {
+if (llvm::sys::fs::exists(Lib)) {
+  ArchiveOfBundles = Lib;
+  FoundAOB = true;
 }
+  } else {
+for (auto LPath : LibraryPaths) {
+  ArchiveOfBundles.clear();
+
+  for (auto Prefix : {"/libdevice/", "/"}) {
+if (Lib.startswith(":"))
+  AOBFileNames.push_back(
+  Twine(LPath + Prefix + Lib.drop_front()).str());
+else if (IsMSVC)
+  AOBFileNames.push_back(Twine(LPath + Prefix + Lib + Ext).str());
+else
+  AOBFileNames.push_back(
+  Twin

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This is awesome, thank you for working on it! @to268 and I had a really good 
discussion during my office hours and what we decided for next steps were:

0) Next time you upload a patch, please use `-U` to give the patch more 
context for reviewers to see.

1. Need to handle function pointers: `auto (*)(void)` and `void (*)(auto)` 
should both be rejected - you may want to consider adding logic to 
`Sema::BuildPointerType()` to semantically restrict forming such a function 
pointer type.
2. Need a ton of random tests:

  auto file_scope_1 = 12;
  // This  be something we accept because the redundant storage class 
specifier is not
  // specifying automatic storage duration due to the lack of a type specifier, 
or we might reject
  // because of the redundant storage class specifier. I have a question out to 
the author to clarify.
  auto auto file_scope_2 = 12;
  
  void func(void) {
int i;
_Generic(i, auto : 0); // Should reject auto in an association
_Alignof(auto); // Should also reject
_Alignas(auto); // Should also reject
  
  
auto j = ({ auto x = 12; x; }); // Should work, x and j should both be int
auto auto k = 12; // 99% this is intended to be accepted; first `auto` is 
the storage class specifier, second `auto` is a redundant storage class 
specifier
  
const int ci = 12;
auto yup = ci;
yup = 12; // Okay, the type of yup is int, not const int
  
_Atomic(auto) atom1 = 12; // Should reject, asking WG14 about it
_Atomic auto atom2 = 12; // Should also reject
  }
  `

It's worth noting that `auto` in C is a very, very strange beast. It's a 
storage-class-specifier, not a type-name. If there's a storage class specifier 
and NO TYPE NAME, then the type is inferred. So grammar productions that use 
type-name without also using `storage-class-specifier` should reject any use of 
`auto` because that's not a valid type name.




Comment at: clang/test/C/C2X/n3007.c:6
+ */
+#include 
+

Rather than include the system header, you should use `_Alignas` and `_Alignof` 
spellings.



Comment at: clang/test/C/C2X/n3007.c:45
+  struct B { auto b; };   // expected-error {{'auto' not allowed in struct 
member}}
+  typedef auto auto_type; // expected-error {{'auto' not allowed in typedef}}
+}

Heh, this doesn't have much to do with structs. :-)



Comment at: clang/test/C/C2X/n3007.c:50
+  auto test_char = 'A';
+  auto test_char_ptr = "test";
+  auto something; // expected-error {{declaration of variable 'something' with 
deduced type 'auto' requires an initializer}}

An additional tests here that's interesting is:
```
_Static_assert(_Generic(test_char_ptr, const char * : 1, char * : 2) == 2, "C 
is weird");
```



Comment at: clang/test/Parser/c2x-auto.c:9
+int a;
+int b;
+union {

Should also test a structure member just to have the coverage.



Comment at: clang/test/Sema/c2x-auto.c:3
+
+#include 
+

Same suggestion here as above.



Comment at: clang/test/Sema/c2x-auto.c:11-20
+void test_structs(void) {
+  struct s_auto { auto a; };  // expected-error {{'auto' not allowed in 
struct member}}
+  auto s_int = (struct { int a; } *)0;
+  typedef auto auto_type; // expected-error {{'auto' not allowed in 
typedef}}
+}
+
+void test_sizeof_alignas(void) {

This seems to be duplicated from the parser tests -- there is only a need to 
test this once (and in each of these cases it seems to be a parsing 
restriction, so the tests should live in Parser).



Comment at: clang/test/Sema/c2x-auto.c:32
+  auto double_cl = (double){2.5};
+  auto auto_cl = (auto){13};  // expected-error {{expected expression}}
+  auto array[] = { 1, 2, 3 }; // expected-error {{'array' declared as array of 
'auto'}}

This should have a FIXME comment -- we should produce a better diagnostic here. 
That we give an error is correct though!

C2x 6.5.2.5p4 specifies that compound literal is rewritten as if it were a 
declaration: `SC typeof(T) ID = { IL };` (where SC is the storage class, T is 
the type, ID is a program-wide unique ID, and IL is the initializer list).

C2x 6.7.2.5p1 only allows `typeof` on an `expression` or a `type-name`, and 
`type-name` does not include `auto` as an option.

So the rewrite makes this invalid.



Comment at: clang/www/c_status.html:1196
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3007.htm";>N3007
-  No
+  Yes
 

This should use class `unreleased` and specify `Clang 16` instead of `Yes` 
("Yes" basically means "we have no idea when we started supporting something, 
we've probably supported this forever")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D1332

[PATCH] D133705: [HIP] Fix unbundling archive

[Yaxun Liu via Phabricator via cfe-commits]

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

remove debug output


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

https://reviews.llvm.org/D133705

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/hip-link-bundle-archive.hip

Index: clang/test/Driver/hip-link-bundle-archive.hip
===
--- clang/test/Driver/hip-link-bundle-archive.hip
+++ clang/test/Driver/hip-link-bundle-archive.hip
@@ -7,7 +7,17 @@
 // RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
 // RUN:   --target=x86_64-unknown-linux-gnu \
 // RUN:   -nogpulib %s -fgpu-rdc -L%T -lhipBundled \
-// RUN:   2>&1 | FileCheck -check-prefix=GNU %s
+// RUN:   2>&1 | FileCheck -check-prefixes=GNU,GNU-L %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-unknown-linux-gnu \
+// RUN:   -nogpulib %s -fgpu-rdc -L%T -l:libhipBundled.a \
+// RUN:   2>&1 | FileCheck -check-prefixes=GNU,GNU-LA %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-unknown-linux-gnu \
+// RUN:   -nogpulib %s -fgpu-rdc %T/libhipBundled.a \
+// RUN:   2>&1 | FileCheck -check-prefixes=GNU,GNU-A %s
 
 // RUN: touch %T/hipBundled2.lib
 // RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
@@ -15,14 +25,26 @@
 // RUN:   -nogpulib %s -fgpu-rdc -L%T -lhipBundled2 \
 // RUN:   2>&1 | FileCheck -check-prefix=MSVC %s
 
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-pc-windows-msvc \
+// RUN:   -nogpulib %s -fgpu-rdc -L%T -l:hipBundled2.lib \
+// RUN:   2>&1 | FileCheck -check-prefix=MSVC %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-pc-windows-msvc \
+// RUN:   -nogpulib %s -fgpu-rdc %T/hipBundled2.lib \
+// RUN:   2>&1 | FileCheck -check-prefix=MSVC %s
+
 // GNU: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libhipBundled.a" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
 // GNU: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx1030" {{.*}} "[[A1030]]"
 // GNU: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libhipBundled.a" "-targets=hip-amdgcn-amd-amdhsa-gfx906" "-output=[[A906:.*\.a]]" "-allow-missing-bundles"
 // GNU: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx906" {{.*}} "[[A906]]"
-// GNU: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-lhipBundled"
+// GNU-L: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-lhipBundled"
+// GNU-LA: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-l:libhipBundled.a"
+// GNU-A: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" "{{.*}}libhipBundled.a"
 
 // MSVC: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}hipBundled2.lib" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
 // MSVC: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx1030" {{.*}} "[[A1030]]"
 // MSVC: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}hipBundled2.lib" "-targets=hip-amdgcn-amd-amdhsa-gfx906" "-output=[[A906:.*\.a]]" "-allow-missing-bundles"
 // MSVC: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx906" {{.*}} "[[A906]]"
-// MSVC: "{{.*}}link{{.*}}" {{.*}}"-out:a.exe" {{.*}}"hipBundled2.lib"
+// MSVC: "{{.*}}link{{.*}}" {{.*}}"-out:a.exe" {{.*}}hipBundled2.lib"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1924,83 +1924,101 @@
   bool FoundAOB = false;
   SmallVector AOBFileNames;
   std::string ArchiveOfBundles;
-  for (auto LPath : LibraryPaths) {
-ArchiveOfBundles.clear();
-
-llvm::Triple Triple(D.getTargetTriple());
-bool IsMSVC = Triple.isWindowsMSVCEnvironment();
-for (auto Prefix : {"/libdevice/", "/"}) {
-  if (IsMSVC)
-AOBFileNames.push_back(Twine(LPath + Prefix + Lib + ".lib").str());
-  AOBFileNames.push_back(Twine(LPath + Prefix + "lib" + Lib + ".a").str());
+
+  llvm::Triple Triple(D.getTargetTriple());
+  bool IsMSVC = Triple.isWindowsMSVCEnvironment();
+  auto Ext = IsMSVC ? ".lib" : ".a";
+  if (Lib.endswith(Ext) && !Lib.startswith(":")) {
+if (llvm::sys::fs::exists(Lib)) {
+  ArchiveOfBundles = Lib;
+  FoundAOB = true;
 }
+  } else {
+for (auto LPath : LibraryPaths) {
+  ArchiveOfBundles.clear();
+
+  for (auto Prefix : {"/libdevice/", "/"}) {
+if (Lib.startswith(":"))
+  AOBFileNames.push_back(
+  Twine(LPath + Prefix + Lib.drop_front()).str());
+else if (IsMSVC)
+  AOBFileNames.push_back(Twine(LPath + Prefix + Lib + Ext).str());
+else
+  AOBFileNames.push_back(
+  Twine(LPath + Prefix + "lib" + Lib + Ext).str());
+  }
 
-for (auto AOB : AOBFileNames) {
-

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/test/Sema/c2x-auto.c:49
+auto b = 9;
+auto c = a + b;
+  }

When I made the comment about the example from the proposal, this was what I 
was thinking about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

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

Re: [clang] b7a7aee - [clang] Qualify auto in range-based for loops (NFC)

[David Blaikie via cfe-commits]

On Sat, Sep 10, 2022 at 3:01 PM Kazu Hirata  wrote:
>
> Thank you Aaron and David for your inputs.
>
> First and foremost, I apologize if I made your job harder by increasing the 
> number of commits you have to peel to get to the real author.
>
> I hear that we are moving toward github pull requests.  A casual search tells 
> me that there are some add-ons to integrate clang-tidy into the code review 
> platform, so I am hoping we can use something like that to get each patch 
> right first time.
>
> Going forward, I'll take git churn and the difficulty of backsliding as big 
> factors in doing future clenaups.  For example, it's probably a good idea to 
> delete a function that hasn't been used for many years (excluding dump 
> functions and such).  Library standardization (like the recent removal of 
> llvm::array_lengthof in favor of std::size) is less good in terms of git 
> churn, but it's very unlikely for somebody to re-introduce 
> llvm::array_lengthof.

I think API deprecations (where the API can be completely removed
eventually, and marked as deprecated/all in-tree usage removed within
O(weeks/months)) especially for cases like the core/support libraries
with relatively many uses, and relatively small APIs are great - if we
get into the territory of naming convention cleanup, that gets more
debatable because there's wide APIs with many naming violations and
then we need more community discussion about what direction we're
going (there are multiple lingering naming conventions, some
discussions about moving to different ones in the future, so maybe
churn to meet the current letter of the style guide would be better
spent after changing the style guide with those directions in mind,
etc).

For stylistic things like range-based-for conversion, const auto*, etc
- yeah, there's some wriggle room depending on how uncontentious the
conversion is, I think.

(there's also some way to mark certain changes as ignorable by git?
Maybe using that more frequently would help lower the cost of these
sort of changes - broader discussion on discourse about ways to
enable/lower the cost of these sort of changes would probably be good
- I think as much as we can make these sort of changes cheaper/less
problematic, to make them more encouraged, is a really good thing to
do)

>
> Thanks,
>
> Kazu Hirata
>
>
> On Fri, Sep 9, 2022 at 5:27 AM Aaron Ballman  wrote:
>>
>> On Thu, Sep 8, 2022 at 12:37 PM David Blaikie  wrote:
>> >
>> > Mixed feelings here - Kazu's made a lot of cleanup/stylistic changes
>> > across the LLVM project for a while now, most, at least I think, are
>> > quite welcome (things like switching to range-based-for, std
>> > algorithms over llvm ones, llvm algorithms over manually written
>> > loops, etc). But yeah, there's some threshold below which the churn
>> > might not be worth the benefit - especially if the change doesn't come
>> > along with tooling to enforce the invariant is maintained in the
>> > future (if it's easy to make mistakes like this one - we'll regress it
>> > and need to do cleanup again in the future)
>>
>> Thanks for speaking up, because I also waffled a bit on whether I
>> called this out or not. :-)
>>
>> > Also, for this particular one, I wonder if in some cases this sort of
>> > automatic transformation isn't ideal - if something is a pointer, but
>> > that's an implementation detail, rather than an intentional feature of
>> > an API (eg: the pointer-ness might be hidden behind a typedef and used
>> > as an opaque handle, without any dereferencing, etc)
>>
>> Agreed.
>>
>> > I think it'd be really good to have some discussion on discourse about
>> > if/how some of these cleanups could be formed into a way to
>> > enforce/encourage the invariant to be maintained going forward -
>> > clang-tidy (assuming that's the basis for the tooling Kazu's using to
>> > make these changes in the first place) integration into the LLVM build
>> > in some way, etc.
>>
>> I think that's a good idea! We want to encourage cleanups, but we
>> don't want to encourage churn, and I think it's somewhat subjective
>> where to draw that line. Having some more community awareness around
>> that would be beneficial. I'm especially interested in how we balance
>> between making incremental style improvements to the project and
>> keeping our git blame logs useful. I'm seeing a lot more git blames
>> that require several steps to get to an interesting commit because of
>> the number of NFCs and reverts/recommits. Unfortunately, the tooling
>> around viewing git blames of large files (like Clang tends to have)
>> makes these sorts of commits surprisingly painful when you do have to
>> dig to see where changes came from. (So I find myself having far less
>> concern when TransGCAttrs.cpp (~350LoC) gets a cleanup like this
>> compared to SemaExpr.cpp (~21kLoC), which suggests to me we should
>> maybe strongly consider splitting more of these massive files up so
>> that churn is less painful.)
>>
>> > & yeah, 

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

FYI: regarding `auto auto`, I've heard back from the author of N3007 and the 
intent was to disallow this construct ("others" was meant to exclude `auto` and 
"except typedef" was meant to add onto the list of exclusions).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

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

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

One thought that occurred to me is that the way C has this specified is awfully 
effectively the same way implicit int worked. It may be worth exploring a 
change to our implicit int functionality to instead generate an implicit `auto` 
type when in C2x mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

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

[PATCH] D133705: [HIP] Fix unbundling archive

[Siu Chi Chan via Phabricator via cfe-commits]

scchan requested changes to this revision.
scchan added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1959
+  if (FoundAOB)
+break;
 }

The AOBFileNames small vector needs to be cleared if !FoundAOB or just move its 
declaration into the LibraryPaths loop.


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

https://reviews.llvm.org/D133705

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

[PATCH] D133611: [clang] sort additional module maps when serializing

[Richard Howell via Phabricator via cfe-commits]

rmaz updated this revision to Diff 459492.
rmaz added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133611

Files:
  clang/lib/Serialization/ASTWriter.cpp


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1282,7 +1282,12 @@
 if (auto *AdditionalModMaps =
 Map.getAdditionalModuleMapFiles(WritingModule)) {
   Record.push_back(AdditionalModMaps->size());
-  for (const FileEntry *F : *AdditionalModMaps)
+  SmallVector ModMaps(AdditionalModMaps->begin(),
+AdditionalModMaps->end());
+  llvm::sort(ModMaps, [](const FileEntry *A, const FileEntry *B) {
+return A->getName() < B->getName();
+  });
+  for (const FileEntry *F : ModMaps)
 AddPath(F->getName(), Record);
 } else {
   Record.push_back(0);


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1282,7 +1282,12 @@
 if (auto *AdditionalModMaps =
 Map.getAdditionalModuleMapFiles(WritingModule)) {
   Record.push_back(AdditionalModMaps->size());
-  for (const FileEntry *F : *AdditionalModMaps)
+  SmallVector ModMaps(AdditionalModMaps->begin(),
+AdditionalModMaps->end());
+  llvm::sort(ModMaps, [](const FileEntry *A, const FileEntry *B) {
+return A->getName() < B->getName();
+  });
+  for (const FileEntry *F : ModMaps)
 AddPath(F->getName(), Record);
 } else {
   Record.push_back(0);
___
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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/include/clang/Format/Format.h:406
+/// Specifies the way to align trailing comments
+TrailingCommentsAlignmentKinds Kind;
+/// How many empty lines to apply alignment

Kind? doesn't feel like the right word



Comment at: clang/include/clang/Format/Format.h:428-433
+bool operator==(const TrailingCommentsAlignmentStyle &R) const {
+  return Kind == R.Kind && OverEmptyLines == R.OverEmptyLines;
+}
+bool operator!=(const TrailingCommentsAlignmentStyle &R) const {
+  return !(*this == R);
+}

yusuke-kadowaki wrote:
> > I don't understand the need for state as a struct could have multiple 
> > options (as enums) each enum should have a state that means "Leave"
> 
> @MyDeveloperDay 
> Without having state, how can this be implemented?
bool Enabled  =  (Kind != FormatStyle::TCAS_Leave)


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

[clang] d96f526 - [clang][test] Disallow using the default module cache path in lit tests

[Ben Langmuir via cfe-commits]

Author: Ben Langmuir
Date: 2022-09-12T09:54:56-07:00
New Revision: d96f526196ac4cebfdd318473816f6d4b9d76707

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

LOG: [clang][test] Disallow using the default module cache path in lit tests

Make the default module cache path invalid when running lit tests so
that tests are forced to provide a cache path. This avoids accidentally
escaping to the system default location, and would have caught the
failure recently found in ClangScanDeps/multiple-commands.c.

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

Added: 


Modified: 
clang/test/Driver/modules-cache-path.m
clang/test/Modules/driver.c
llvm/utils/lit/lit/llvm/config.py

Removed: 




diff  --git a/clang/test/Driver/modules-cache-path.m 
b/clang/test/Driver/modules-cache-path.m
index 1da27d2143631..f700a9738742f 100644
--- a/clang/test/Driver/modules-cache-path.m
+++ b/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 -u CLANG_MODULE_CACHE_PATH %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 \

diff  --git a/clang/test/Modules/driver.c b/clang/test/Modules/driver.c
index 34fc163a5ccd4..8ffa23ba4e71c 100644
--- a/clang/test/Modules/driver.c
+++ b/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 -u CLANG_MODULE_CACHE_PATH %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"}}

diff  --git a/llvm/utils/lit/lit/llvm/config.py 
b/llvm/utils/lit/lit/llvm/config.py
index 591b9938f211f..6149f4db7e0ca 100644
--- a/llvm/utils/lit/lit/llvm/config.py
+++ b/llvm/utils/lit/lit/llvm/config.py
@@ -495,6 +495,10 @@ def use_clang(self, additional_tool_dirs=[], 
additional_flags=[],
 
 self.clear_environment(possibly_dangerous_env_vars)
 
+# Make the default module cache path invalid so that tests are forced 
to
+# provide a cache path if they use implicit modules.
+self.with_environment('CLANG_MODULE_CACHE_PATH', '/dev/null')
+
 # Tweak the PATH to include the tools dir and the scripts dir.
 # Put Clang first to avoid LLVM from overriding out-of-tree clang
 # builds.



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

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

[Ben Langmuir via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd96f526196ac: [clang][test] Disallow using the default 
module cache path in lit tests (authored by benlangmuir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133622

Files:
  clang/test/Driver/modules-cache-path.m
  clang/test/Modules/driver.c
  llvm/utils/lit/lit/llvm/config.py


Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -495,6 +495,10 @@
 
 self.clear_environment(possibly_dangerous_env_vars)
 
+# Make the default module cache path invalid so that tests are forced 
to
+# provide a cache path if they use implicit modules.
+self.with_environment('CLANG_MODULE_CACHE_PATH', '/dev/null')
+
 # Tweak the PATH to include the tools dir and the scripts dir.
 # Put Clang first to avoid LLVM from overriding out-of-tree clang
 # builds.
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 -u CLANG_MODULE_CACHE_PATH %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 -u CLANG_MODULE_CACHE_PATH %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: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -495,6 +495,10 @@
 
 self.clear_environment(possibly_dangerous_env_vars)
 
+# Make the default module cache path invalid so that tests are forced to
+# provide a cache path if they use implicit modules.
+self.with_environment('CLANG_MODULE_CACHE_PATH', '/dev/null')
+
 # Tweak the PATH to include the tools dir and the scripts dir.
 # Put Clang first to avoid LLVM from overriding out-of-tree clang
 # builds.
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 -u CLANG_MODULE_CACHE_PATH %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 -u CLANG_MODULE_CACHE_PATH %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

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

[Yusuke Kadowaki via Phabricator via cfe-commits]

yusuke-kadowaki marked 2 inline comments as done.
yusuke-kadowaki added a comment.

So other than the naming, does the struct look good?




Comment at: clang/include/clang/Format/Format.h:406
+/// Specifies the way to align trailing comments
+TrailingCommentsAlignmentKinds Kind;
+/// How many empty lines to apply alignment

MyDeveloperDay wrote:
> Kind? doesn't feel like the right word
What do you recommend?



Comment at: clang/include/clang/Format/Format.h:428-433
+bool operator==(const TrailingCommentsAlignmentStyle &R) const {
+  return Kind == R.Kind && OverEmptyLines == R.OverEmptyLines;
+}
+bool operator!=(const TrailingCommentsAlignmentStyle &R) const {
+  return !(*this == R);
+}

MyDeveloperDay wrote:
> yusuke-kadowaki wrote:
> > > I don't understand the need for state as a struct could have multiple 
> > > options (as enums) each enum should have a state that means "Leave"
> > 
> > @MyDeveloperDay 
> > Without having state, how can this be implemented?
> bool Enabled  =  (Kind != FormatStyle::TCAS_Leave)
Oh ok so I think we are on the same page.


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] D133694: [Clang][OpenMP] Fix use_device_addr

[Ye Luo via Phabricator via cfe-commits]

ye-luo added inline comments.



Comment at: clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp:14
+{
+#pragma omp target data use_device_addr(x)
+{

In my understanding of the spec.
`map(tofrom:x[0:256])` only maps the memory segment that x points to. x itself 
as a pointer scalar is not mapped.
use_device_addr(x) should fail to find the map of x scalar.
5.2 spec.
If the list item is not a mapped list item, it is assumed to be accessible on 
the target device.
To me, it seems just keep &x as it was, in this case &x remains a host address.

But in your patch description, it seems treating x differently from a scalar.

I also applied your patch on main and got segfault because the x has a value of 
device address and x[0] fails. This should be the behavior of use_device_ptr 
instead of use_device_addr.


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

https://reviews.llvm.org/D133694

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

[PATCH] D133705: [HIP] Fix unbundling archive

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1959
+  if (FoundAOB)
+break;
 }

scchan wrote:
> The AOBFileNames small vector needs to be cleared if !FoundAOB or just move 
> its declaration into the LibraryPaths loop.
> The AOBFileNames small vector needs to be cleared if !FoundAOB or just move 
> its declaration into the LibraryPaths loop.

will move the decl into LibraryPaths loop.


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

https://reviews.llvm.org/D133705

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

[PATCH] D133705: [HIP] Fix unbundling archive

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 459502.
yaxunl marked an inline comment as done.
yaxunl added a comment.

revised by Siu Chi's comments


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

https://reviews.llvm.org/D133705

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/hip-link-bundle-archive.hip

Index: clang/test/Driver/hip-link-bundle-archive.hip
===
--- clang/test/Driver/hip-link-bundle-archive.hip
+++ clang/test/Driver/hip-link-bundle-archive.hip
@@ -7,7 +7,17 @@
 // RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
 // RUN:   --target=x86_64-unknown-linux-gnu \
 // RUN:   -nogpulib %s -fgpu-rdc -L%T -lhipBundled \
-// RUN:   2>&1 | FileCheck -check-prefix=GNU %s
+// RUN:   2>&1 | FileCheck -check-prefixes=GNU,GNU-L %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-unknown-linux-gnu \
+// RUN:   -nogpulib %s -fgpu-rdc -L%T -l:libhipBundled.a \
+// RUN:   2>&1 | FileCheck -check-prefixes=GNU,GNU-LA %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-unknown-linux-gnu \
+// RUN:   -nogpulib %s -fgpu-rdc %T/libhipBundled.a \
+// RUN:   2>&1 | FileCheck -check-prefixes=GNU,GNU-A %s
 
 // RUN: touch %T/hipBundled2.lib
 // RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
@@ -15,14 +25,26 @@
 // RUN:   -nogpulib %s -fgpu-rdc -L%T -lhipBundled2 \
 // RUN:   2>&1 | FileCheck -check-prefix=MSVC %s
 
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-pc-windows-msvc \
+// RUN:   -nogpulib %s -fgpu-rdc -L%T -l:hipBundled2.lib \
+// RUN:   2>&1 | FileCheck -check-prefix=MSVC %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-pc-windows-msvc \
+// RUN:   -nogpulib %s -fgpu-rdc %T/hipBundled2.lib \
+// RUN:   2>&1 | FileCheck -check-prefix=MSVC %s
+
 // GNU: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libhipBundled.a" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
 // GNU: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx1030" {{.*}} "[[A1030]]"
 // GNU: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libhipBundled.a" "-targets=hip-amdgcn-amd-amdhsa-gfx906" "-output=[[A906:.*\.a]]" "-allow-missing-bundles"
 // GNU: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx906" {{.*}} "[[A906]]"
-// GNU: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-lhipBundled"
+// GNU-L: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-lhipBundled"
+// GNU-LA: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-l:libhipBundled.a"
+// GNU-A: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" "{{.*}}libhipBundled.a"
 
 // MSVC: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}hipBundled2.lib" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
 // MSVC: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx1030" {{.*}} "[[A1030]]"
 // MSVC: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}hipBundled2.lib" "-targets=hip-amdgcn-amd-amdhsa-gfx906" "-output=[[A906:.*\.a]]" "-allow-missing-bundles"
 // MSVC: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx906" {{.*}} "[[A906]]"
-// MSVC: "{{.*}}link{{.*}}" {{.*}}"-out:a.exe" {{.*}}"hipBundled2.lib"
+// MSVC: "{{.*}}link{{.*}}" {{.*}}"-out:a.exe" {{.*}}hipBundled2.lib"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1922,85 +1922,103 @@
 return false;
 
   bool FoundAOB = false;
-  SmallVector AOBFileNames;
   std::string ArchiveOfBundles;
-  for (auto LPath : LibraryPaths) {
-ArchiveOfBundles.clear();
-
-llvm::Triple Triple(D.getTargetTriple());
-bool IsMSVC = Triple.isWindowsMSVCEnvironment();
-for (auto Prefix : {"/libdevice/", "/"}) {
-  if (IsMSVC)
-AOBFileNames.push_back(Twine(LPath + Prefix + Lib + ".lib").str());
-  AOBFileNames.push_back(Twine(LPath + Prefix + "lib" + Lib + ".a").str());
+
+  llvm::Triple Triple(D.getTargetTriple());
+  bool IsMSVC = Triple.isWindowsMSVCEnvironment();
+  auto Ext = IsMSVC ? ".lib" : ".a";
+  if (Lib.endswith(Ext) && !Lib.startswith(":")) {
+if (llvm::sys::fs::exists(Lib)) {
+  ArchiveOfBundles = Lib;
+  FoundAOB = true;
 }
+  } else {
+for (auto LPath : LibraryPaths) {
+  ArchiveOfBundles.clear();
+  SmallVector AOBFileNames;
+
+  for (auto Prefix : {"/libdevice/", "/"}) {
+if (Lib.startswith(":"))
+  AOBFileNames.push_back(
+  Twine(LPath + Prefix + Lib.drop_front()).str());
+else if (IsMSVC)
+  AOBFileNames.push_back(Twine(LPath + Prefix + Lib + Ext).str());
+else
+  AOBFileNames.push_back(
+

[clang] 7eead18 - [Clang] NFC: Make UnqualifiedId::Kind private for consistency.

[Corentin Jabot via cfe-commits]

Author: Corentin Jabot
Date: 2022-09-12T19:14:06+02:00
New Revision: 7eead180b92dcff7eaf9d71df14e33936ec4dbe5

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

LOG: [Clang] NFC: Make UnqualifiedId::Kind private for consistency.

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

Added: 


Modified: 
clang/include/clang/Sema/DeclSpec.h
clang/lib/Sema/DeclSpec.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/DeclSpec.h 
b/clang/include/clang/Sema/DeclSpec.h
index bb49151c77248..1aa335b8b9b7a 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -964,10 +964,10 @@ class UnqualifiedId {
   UnqualifiedId(const UnqualifiedId &Other) = delete;
   const UnqualifiedId &operator=(const UnqualifiedId &) = delete;
 
-public:
   /// Describes the kind of unqualified-id parsed.
   UnqualifiedIdKind Kind;
 
+public:
   struct OFI {
 /// The kind of overloaded operator.
 OverloadedOperatorKind Operator;

diff  --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp
index 6875d3d97c282..0c4b79d8b9642 100644
--- a/clang/lib/Sema/DeclSpec.cpp
+++ b/clang/lib/Sema/DeclSpec.cpp
@@ -413,7 +413,7 @@ bool Declarator::isDeclarationOfFunction() const {
 bool Declarator::isStaticMember() {
   assert(getContext() == DeclaratorContext::Member);
   return getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_static ||
- (getName().Kind == UnqualifiedIdKind::IK_OperatorFunctionId &&
+ (getName().getKind() == UnqualifiedIdKind::IK_OperatorFunctionId &&
   CXXMethodDecl::isStaticOverloadedOperator(
   getName().OperatorFunctionId.Operator));
 }

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 6e94da4a115eb..9d0e6769f0875 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6586,8 +6586,8 @@ Sema::ActOnTypedefDeclarator(Scope* S, Declarator& D, 
DeclContext* DC,
 Diag(D.getDeclSpec().getConstexprSpecLoc(), diag::err_invalid_constexpr)
 << 1 << static_cast(D.getDeclSpec().getConstexprSpecifier());
 
-  if (D.getName().Kind != UnqualifiedIdKind::IK_Identifier) {
-if (D.getName().Kind == UnqualifiedIdKind::IK_DeductionGuideName)
+  if (D.getName().getKind() != UnqualifiedIdKind::IK_Identifier) {
+if (D.getName().getKind() == UnqualifiedIdKind::IK_DeductionGuideName)
   Diag(D.getName().StartLocation,
diag::err_deduction_guide_invalid_specifier)
   << "typedef";

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9a2ee556fc2e0..6463d0678007d 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -13009,7 +13009,7 @@ Decl *Sema::ActOnAliasDeclaration(Scope *S, 
AccessSpecifier AS,
 Previous.clear();
   }
 
-  assert(Name.Kind == UnqualifiedIdKind::IK_Identifier &&
+  assert(Name.getKind() == UnqualifiedIdKind::IK_Identifier &&
  "name in alias declaration must be an identifier");
   TypeAliasDecl *NewTD = TypeAliasDecl::Create(Context, CurContext, UsingLoc,
Name.StartLocation,



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

[PATCH] D133703: [Clang] NFC: Make UnqualifiedId::Kind private for consistency.

[Corentin Jabot via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7eead180b92d: [Clang] NFC: Make UnqualifiedId::Kind private 
for consistency. (authored by cor3ntin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133703

Files:
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13009,7 +13009,7 @@
 Previous.clear();
   }
 
-  assert(Name.Kind == UnqualifiedIdKind::IK_Identifier &&
+  assert(Name.getKind() == UnqualifiedIdKind::IK_Identifier &&
  "name in alias declaration must be an identifier");
   TypeAliasDecl *NewTD = TypeAliasDecl::Create(Context, CurContext, UsingLoc,
Name.StartLocation,
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6586,8 +6586,8 @@
 Diag(D.getDeclSpec().getConstexprSpecLoc(), diag::err_invalid_constexpr)
 << 1 << static_cast(D.getDeclSpec().getConstexprSpecifier());
 
-  if (D.getName().Kind != UnqualifiedIdKind::IK_Identifier) {
-if (D.getName().Kind == UnqualifiedIdKind::IK_DeductionGuideName)
+  if (D.getName().getKind() != UnqualifiedIdKind::IK_Identifier) {
+if (D.getName().getKind() == UnqualifiedIdKind::IK_DeductionGuideName)
   Diag(D.getName().StartLocation,
diag::err_deduction_guide_invalid_specifier)
   << "typedef";
Index: clang/lib/Sema/DeclSpec.cpp
===
--- clang/lib/Sema/DeclSpec.cpp
+++ clang/lib/Sema/DeclSpec.cpp
@@ -413,7 +413,7 @@
 bool Declarator::isStaticMember() {
   assert(getContext() == DeclaratorContext::Member);
   return getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_static ||
- (getName().Kind == UnqualifiedIdKind::IK_OperatorFunctionId &&
+ (getName().getKind() == UnqualifiedIdKind::IK_OperatorFunctionId &&
   CXXMethodDecl::isStaticOverloadedOperator(
   getName().OperatorFunctionId.Operator));
 }
Index: clang/include/clang/Sema/DeclSpec.h
===
--- clang/include/clang/Sema/DeclSpec.h
+++ clang/include/clang/Sema/DeclSpec.h
@@ -964,10 +964,10 @@
   UnqualifiedId(const UnqualifiedId &Other) = delete;
   const UnqualifiedId &operator=(const UnqualifiedId &) = delete;
 
-public:
   /// Describes the kind of unqualified-id parsed.
   UnqualifiedIdKind Kind;
 
+public:
   struct OFI {
 /// The kind of overloaded operator.
 OverloadedOperatorKind Operator;


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13009,7 +13009,7 @@
 Previous.clear();
   }
 
-  assert(Name.Kind == UnqualifiedIdKind::IK_Identifier &&
+  assert(Name.getKind() == UnqualifiedIdKind::IK_Identifier &&
  "name in alias declaration must be an identifier");
   TypeAliasDecl *NewTD = TypeAliasDecl::Create(Context, CurContext, UsingLoc,
Name.StartLocation,
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6586,8 +6586,8 @@
 Diag(D.getDeclSpec().getConstexprSpecLoc(), diag::err_invalid_constexpr)
 << 1 << static_cast(D.getDeclSpec().getConstexprSpecifier());
 
-  if (D.getName().Kind != UnqualifiedIdKind::IK_Identifier) {
-if (D.getName().Kind == UnqualifiedIdKind::IK_DeductionGuideName)
+  if (D.getName().getKind() != UnqualifiedIdKind::IK_Identifier) {
+if (D.getName().getKind() == UnqualifiedIdKind::IK_DeductionGuideName)
   Diag(D.getName().StartLocation,
diag::err_deduction_guide_invalid_specifier)
   << "typedef";
Index: clang/lib/Sema/DeclSpec.cpp
===
--- clang/lib/Sema/DeclSpec.cpp
+++ clang/lib/Sema/DeclSpec.cpp
@@ -413,7 +413,7 @@
 bool Declarator::isStaticMember() {
   assert(getContext() == DeclaratorContext::Member);
   return getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_static ||
- (getName().Kind == UnqualifiedIdKind::IK_OperatorFunctionId &&
+ (getName().getKind() == UnqualifiedIdKind::IK_OperatorFunctionId &&
   CXXMethodDecl::isStaticOverloadedOperator(
   getName().OperatorFunctionId.Operator));
 }
Index: clang/inclu

[PATCH] D133711: [Sema] Reject array element types whose alignments are larger than their sizes

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: efriedma, rjmccall, rnk, sepavloff.
ahatanak added a project: clang.
Herald added a project: All.
ahatanak requested review of this revision.

In the following code, the first element is aligned on a 16-byte boundary, but 
the remaining elements aren't:

  typedef char int8_a16 __attribute__((aligned(16)));
  int8_a16 array[4];

Currently clang doesn't reject the code, but it should since it can cause 
crashes at runtime. This patch also fixes assertion failures in CodeGen caused 
by the changes in https://reviews.llvm.org/D123649.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133711

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenCXX/ubsan-new-checks.cpp
  clang/test/Layout/ms-aligned-array.c
  clang/test/Sema/attr-aligned.c
  clang/test/SemaCXX/array-alignment.cpp
  clang/test/SemaCXX/warn-new-overaligned.cpp

Index: clang/test/SemaCXX/warn-new-overaligned.cpp
===
--- clang/test/SemaCXX/warn-new-overaligned.cpp
+++ clang/test/SemaCXX/warn-new-overaligned.cpp
@@ -19,9 +19,13 @@
 }
 
 namespace test2 {
+struct S {
+  char c[256];
+};
+
 class Test {
-  typedef int __attribute__((aligned(256))) aligned_int;
-  aligned_int high_contention_data[10];
+  typedef S __attribute__((aligned(256))) alignedS;
+  alignedS high_contention_data[10];
 };
 
 void helper() {
Index: clang/test/SemaCXX/array-alignment.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/array-alignment.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+typedef char __attribute__((aligned(2))) AlignedChar;
+typedef AlignedChar arrayType0[4]; // expected-error {{alignment of array element}}
+
+typedef struct __attribute__((aligned(8))) {
+  int m0;
+} AlignedStruct;
+
+AlignedChar a0[1]; // expected-error {{alignment of array element}}
+AlignedStruct a1[1];
+
+struct S {
+  AlignedChar m0[1]; // expected-error {{alignment of array element}}
+  AlignedStruct m1[1];
+};
+
+void test(char *p) {
+  auto p0 = (AlignedChar(*)[1])p;// expected-error {{alignment of array element}}
+  auto r0 = (AlignedChar(&)[1])(*p); // expected-error {{alignment of array element}}
+  auto p1 = new AlignedChar[1];  // expected-error {{alignment of array element}}
+  auto p2 = (AlignedStruct(*)[1])p;
+  auto p3 = new AlignedStruct[1];
+}
Index: clang/test/Sema/attr-aligned.c
===
--- clang/test/Sema/attr-aligned.c
+++ clang/test/Sema/attr-aligned.c
@@ -49,13 +49,14 @@
 char e1[__alignof__(e) == 2 ?: -1] = {0};
 char e2[__alignof__(e.member) == 2 ?: -1] = {0};
 
-typedef char overaligned_char __attribute__((aligned(16)));
-typedef overaligned_char array_with_overaligned_char[11];
+typedef struct { char c[16]; } S;
+typedef S overaligned_struct __attribute__((aligned(16)));
+typedef overaligned_struct array_with_overaligned_struct[11];
 typedef char array_with_align_attr[11] __attribute__((aligned(16)));
 
-char f0[__alignof__(array_with_overaligned_char) == 16 ? 1 : -1] = { 0 };
+char f0[__alignof__(array_with_overaligned_struct) == 16 ? 1 : -1] = { 0 };
 char f1[__alignof__(array_with_align_attr) == 16 ? 1 : -1] = { 0 };
-array_with_overaligned_char F2;
+array_with_overaligned_struct F2;
 char f2[__alignof__(F2) == 16 ? 1 : -1] = { 0 };
 array_with_align_attr F3;
 char f3[__alignof__(F3) == 16 ? 1 : -1] = { 0 };
Index: clang/test/Layout/ms-aligned-array.c
===
--- clang/test/Layout/ms-aligned-array.c
+++ clang/test/Layout/ms-aligned-array.c
@@ -6,7 +6,10 @@
 // FIXME: What about other type sugar, like _Atomic? This would only matter in a
 // packed struct context.
 struct __declspec(align(16)) AlignedStruct { int x; };
-typedef int  __declspec(align(16)) AlignedInt;
+struct Struct2 {
+  char c[16];
+};
+typedef struct Struct2 __declspec(align(16)) AlignedStruct2;
 
 #define CHECK_SIZE(X, Align) \
   _Static_assert(__alignof(struct X) == Align, "should be aligned");
@@ -20,13 +23,13 @@
 
 struct B {
   char b;
-  AlignedInt a[1];
+  AlignedStruct2 a[1];
 };
 CHECK_SIZE(B, 16);
 
 struct C {
   char b;
-  AlignedInt a[];
+  AlignedStruct2 a[];
 };
 CHECK_SIZE(C, 16);
 
@@ -39,14 +42,18 @@
 // CHECK-NEXT:  0 |   struct AlignedStruct[1] a
 // CHECK-NEXT:| [sizeof=16, align=16]
 // CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct Struct2
+// CHECK-NEXT:  0 |   char[16] c
+// CHECK-NEXT:| [sizeof=16, align=1]
+// CHECK: *** Dumping AST Record Layout
 // CHECK-NEXT:  0 | struct B
 // CHECK-NEXT:  0 |   char b
-// CHECK-NEXT: 16 |   AlignedInt[1] a
+// CHECK-NEXT: 16 |   AlignedStruct2[1] a
 // CHECK-NEXT:  

[PATCH] D133325: [Driver] Allow search of included response files as configuration files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 459508.
sepavloff added a comment.

Rebased, addressed reviewers' notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133325

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/config/config-3.cfg
  clang/test/Driver/config-file.c
  clang/unittests/Driver/ToolChainTest.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp

Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1184,6 +1184,13 @@
 
   if (!RelativeNames)
 return Error::success();
+
+  if (InConfigFile &&
+  std::find_if(NewArgv.begin(), NewArgv.end(), [](const char *Arg) -> bool {
+return StringRef("--search-config-dirs") == Arg;
+  }) != NewArgv.end())
+setSearchAsConfig(true);
+
   llvm::StringRef BasePath = llvm::sys::path::parent_path(FName);
   // If names of nested response files should be resolved relative to including
   // file, replace the included response file names with their full paths
@@ -1207,8 +1214,17 @@
 
 SmallString<128> ResponseFile;
 ResponseFile.push_back('@');
-ResponseFile.append(BasePath);
-llvm::sys::path::append(ResponseFile, FileName);
+if (SearchAsConfig) {
+  std::string FilePath;
+  if (!findConfigFile(FileName, FilePath))
+return llvm::createStringError(
+std::make_error_code(std::errc::no_such_file_or_directory),
+"Could not find config file: " + FileName);
+  ResponseFile.append(FilePath);
+} else {
+  ResponseFile.append(BasePath);
+  llvm::sys::path::append(ResponseFile, FileName);
+}
 Arg = Saver.save(ResponseFile.str()).data();
   }
   return Error::success();
@@ -1350,15 +1366,46 @@
llvm::vfs::FileSystem *FS)
 : Saver(S), Tokenizer(T), FS(FS ? FS : vfs::getRealFileSystem().get()) {}
 
-bool ExpansionContext::readConfigFile(StringRef CfgFile,
-  SmallVectorImpl &Argv) {
-  SmallString<128> AbsPath;
-  if (sys::path::is_relative(CfgFile)) {
-AbsPath.assign(CfgFile);
-if (std::error_code EC = FS->makeAbsolute(AbsPath))
+bool ExpansionContext::findConfigFile(StringRef FileName,
+  std::string &FilePath) {
+  SmallString<128> CfgFilePath;
+  const auto FileExists = [this](SmallString<128> Path) -> bool {
+auto Status = FS->status(Path);
+return Status &&
+   Status->getType() == llvm::sys::fs::file_type::regular_file;
+  };
+
+  // If file name contains directory separator, treat it as a path to
+  // configuration file.
+  if (llvm::sys::path::has_parent_path(FileName)) {
+CfgFilePath = FileName;
+if (llvm::sys::path::is_relative(FileName) && FS->makeAbsolute(CfgFilePath))
   return false;
-CfgFile = AbsPath.str();
+if (!FileExists(CfgFilePath))
+  return false;
+FilePath = CfgFilePath.str();
+return true;
   }
+
+  // Look for the file in search directories.
+  for (const StringRef &Dir : SearchDirs) {
+if (Dir.empty())
+  continue;
+CfgFilePath.assign(Dir);
+llvm::sys::path::append(CfgFilePath, FileName);
+llvm::sys::path::native(CfgFilePath);
+if (FileExists(CfgFilePath)) {
+  FilePath = CfgFilePath.str();
+  return true;
+}
+  }
+
+  return false;
+}
+
+bool ExpansionContext::readConfigFile(StringRef CfgFile,
+  SmallVectorImpl &Argv) {
+  assert(llvm::sys::path::is_absolute(CfgFile));
   InConfigFile = true;
   RelativeNames = true;
   if (llvm::Error Err = expandResponseFile(CfgFile, Argv)) {
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -2080,6 +2080,9 @@
   /// current directory is used instead.
   StringRef CurrentDir;
 
+  /// Directories used for search of config files.
+  ArrayRef SearchDirs;
+
   /// True if names of nested response files must be resolved relative to
   /// including file.
   bool RelativeNames = false;
@@ -2091,6 +2094,10 @@
   /// If true, body of config file is expanded.
   bool InConfigFile = false;
 
+  /// If true, the file included by '@file' is searched for as a config file, in
+  /// the config search directories.
+  bool SearchAsConfig = false;
+
   llvm::Error expandResponseFile(StringRef FName,
  SmallVectorImpl &NewArgv);
 
@@ -2113,6 +2120,29 @@
 return *this;
   }
 
+  ArrayRef getSearchDirs() const { return SearchDirs; }
+  ExpansionContext &setSearchDirs(ArrayRef X) {
+SearchDirs = X;
+return *this;
+  }
+
+  bool get

[PATCH] D133694: [Clang][OpenMP] Fix use_device_addr

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp:14
+{
+#pragma omp target data use_device_addr(x)
+{

ye-luo wrote:
> In my understanding of the spec.
> `map(tofrom:x[0:256])` only maps the memory segment that x points to. x 
> itself as a pointer scalar is not mapped.
> use_device_addr(x) should fail to find the map of x scalar.
> 5.2 spec.
> If the list item is not a mapped list item, it is assumed to be accessible on 
> the target device.
> To me, it seems just keep &x as it was, in this case &x remains a host 
> address.
> 
> But in your patch description, it seems treating x differently from a scalar.
> 
> I also applied your patch on main and got segfault because the x has a value 
> of device address and x[0] fails. This should be the behavior of 
> use_device_ptr instead of use_device_addr.
> To me, it seems just keep &x as it was, in this case &x remains a host 
> address.

So does this mean that if I do something like this in the target data I should 
get different addresses for x:


```
#pragma omp target data use_device_ptr(x)
{
fprintf(stderr, "x: %p\n", __LINE__, x);
}

#pragma omp target data use_device_addr(x)
{
fprintf(stderr, "x: %p\n", __LINE__, x);
}
```


> I also applied your patch on main and got segfault because the x has a value 
> of device address and x[0] fails.

That's my fault x[0] was the wrong thing to use actually.




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

https://reviews.llvm.org/D133694

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

[PATCH] D133711: [Sema] Reject array element types whose alignments are larger than their sizes

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a reviewer: aaron.ballman.
efriedma added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:2406
+
+  if (Size.isZero() || Size >= Context.getTypeAlignInChars(EltTy))
+return false;

I think you need to check that the size is a multiple of the alignment, not 
just larger than the alignment.  (Those are only the same thing if the size is 
a power of two; it looks like you don't have any test coverage for 
non-power-of-two sizes.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133711

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

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

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 459519.
python3kgae marked an inline comment as done.
python3kgae added a comment.

Fix test fail caused by not add HLSL builtin Resource type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132421

Files:
  clang/include/clang/Sema/HLSLExternalSemaSource.h
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/test/AST/HLSL/Inputs/pch.hlsl
  clang/test/AST/HLSL/Inputs/pch_with_buf.hlsl
  clang/test/AST/HLSL/pch.hlsl
  clang/test/AST/HLSL/pch_with_buf.hlsl

Index: clang/test/AST/HLSL/pch_with_buf.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/pch_with_buf.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -finclude-default-header -emit-pch -o %t %S/Inputs/pch_with_buf.hlsl
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl \
+// RUN:  -finclude-default-header -include-pch %t -fsyntax-only -ast-dump-all %s | FileCheck  %s
+
+// Make sure PCH works by using function declared in PCH header.
+// CHECK:FunctionDecl 0x[[FOO:[0-9a-f]+]] <{{.*}}:2:1, line:4:1> line:2:8 imported used foo 'float2 (float2, float2)'
+// Make sure buffer defined in PCH works.
+// CHECK:VarDecl 0x{{[0-9a-f]+}}  col:17 imported Buf 'RWBuffer':'hlsl::RWBuffer<>'
+// Make sure declare a RWBuffer in current file works.
+// CHECK:VarDecl 0x{{[0-9a-f]+}} <{{.*}}:11:1, col:23> col:23 Buf2 'hlsl::RWBuffer':'hlsl::RWBuffer<>'
+hlsl::RWBuffer Buf2;
+
+float2 bar(float2 a, float2 b) {
+// CHECK:CallExpr 0x{{[0-9a-f]+}}  'float2':'float __attribute__((ext_vector_type(2)))'
+// CHECK-NEXT:ImplicitCastExpr 0x{{[0-9a-f]+}}  'float2 (*)(float2, float2)' 
+// CHECK-NEXT:`-DeclRefExpr 0x{{[0-9a-f]+}}  'float2 (float2, float2)' lvalue Function 0x[[FOO]] 'foo' 'float2 (float2, float2)'
+  return foo(a, b);
+}
Index: clang/test/AST/HLSL/pch.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/pch.hlsl
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl \
+// RUN:  -finclude-default-header -emit-pch -o %t %S/Inputs/pch.hlsl
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl \
+// RUN:  -finclude-default-header -include-pch %t -fsyntax-only -ast-dump-all %s \
+// RUN: | FileCheck  %s
+
+// Make sure PCH works by using function declared in PCH header and declare a RWBuffer in current file.
+// CHECK:FunctionDecl 0x[[FOO:[0-9a-f]+]] <{{.*}}:2:1, line:4:1> line:2:8 imported used foo 'float2 (float2, float2)'
+// CHECK:VarDecl 0x{{[0-9a-f]+}} <{{.*}}:10:1, col:23> col:23 Buffer 'hlsl::RWBuffer':'hlsl::RWBuffer<>'
+hlsl::RWBuffer Buffer;
+
+float2 bar(float2 a, float2 b) {
+// CHECK:CallExpr 0x{{[0-9a-f]+}}  'float2':'float __attribute__((ext_vector_type(2)))'
+// CHECK-NEXT:ImplicitCastExpr 0x{{[0-9a-f]+}}  'float2 (*)(float2, float2)' 
+// CHECK-NEXT:`-DeclRefExpr 0x{{[0-9a-f]+}}  'float2 (float2, float2)' lvalue Function 0x[[FOO]] 'foo' 'float2 (float2, float2)'
+  return foo(a, b);
+}
Index: clang/test/AST/HLSL/Inputs/pch_with_buf.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/Inputs/pch_with_buf.hlsl
@@ -0,0 +1,6 @@
+
+float2 foo(float2 a, float2 b) {
+  return a + b;
+}
+
+RWBuffer Buf;
Index: clang/test/AST/HLSL/Inputs/pch.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/Inputs/pch.hlsl
@@ -0,0 +1,4 @@
+
+float2 foo(float2 a, float2 b) {
+  return a + b;
+}
Index: clang/lib/Sema/HLSLExternalSemaSource.cpp
===
--- clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -30,6 +30,7 @@
 struct BuiltinTypeDeclBuilder {
   CXXRecordDecl *Record = nullptr;
   ClassTemplateDecl *Template = nullptr;
+  ClassTemplateDecl *PrevTemplate = nullptr;
   NamespaceDecl *HLSLNamespace = nullptr;
   llvm::StringMap Fields;
 
@@ -43,48 +44,46 @@
 ASTContext &AST = S.getASTContext();
 IdentifierInfo &II = AST.Idents.get(Name, tok::TokenKind::identifier);
 
+LookupResult Result(S, &II, SourceLocation(), Sema::LookupTagName);
+CXXRecordDecl *PrevDecl = nullptr;
+if (S.LookupQualifiedName(Result, HLSLNamespace)) {
+  NamedDecl *Found = Result.getFoundDecl();
+  if (auto *TD = dyn_cast(Found)) {
+PrevDecl = TD->getTemplatedDecl();
+PrevTemplate = TD;
+  } else
+PrevDecl = dyn_cast(Found);
+  assert(PrevDecl && "Unexpected lookup result type.");
+}
+
+if (PrevDecl && PrevDecl->isCompleteDefinition()) {
+  Record = PrevDecl;
+  return;
+}
+
 Record = CXXRecordDecl::Create(AST, TagDecl::TagKind::TTK_Class,
HLSLNamespace, SourceLocation(),
-   

[PATCH] D133705: [HIP] Fix unbundling archive

[Siu Chi Chan via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D133705

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

[PATCH] D132643: [OpenMP] Extend lit test for parallel for simd construct

[Saiyedul Islam via Phabricator via cfe-commits]

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

LGTM. Thank!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132643

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

[PATCH] D133694: [Clang][OpenMP] Fix use_device_addr

[Ye Luo via Phabricator via cfe-commits]

ye-luo added inline comments.



Comment at: clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp:14
+{
+#pragma omp target data use_device_addr(x)
+{

doru1004 wrote:
> ye-luo wrote:
> > In my understanding of the spec.
> > `map(tofrom:x[0:256])` only maps the memory segment that x points to. x 
> > itself as a pointer scalar is not mapped.
> > use_device_addr(x) should fail to find the map of x scalar.
> > 5.2 spec.
> > If the list item is not a mapped list item, it is assumed to be accessible 
> > on the target device.
> > To me, it seems just keep &x as it was, in this case &x remains a host 
> > address.
> > 
> > But in your patch description, it seems treating x differently from a 
> > scalar.
> > 
> > I also applied your patch on main and got segfault because the x has a 
> > value of device address and x[0] fails. This should be the behavior of 
> > use_device_ptr instead of use_device_addr.
> > To me, it seems just keep &x as it was, in this case &x remains a host 
> > address.
> 
> So does this mean that if I do something like this in the target data I 
> should get different addresses for x:
> 
> 
> ```
> #pragma omp target data use_device_ptr(x)
> {
> fprintf(stderr, "x: %p\n", __LINE__, x);
> }
> 
> #pragma omp target data use_device_addr(x)
> {
> fprintf(stderr, "x: %p\n", __LINE__, x);
> }
> ```
> 
> 
> > I also applied your patch on main and got segfault because the x has a 
> > value of device address and x[0] fails.
> 
> That's my fault x[0] was the wrong thing to use actually.
> 
> 
When you have an outer `target data map(x)`, then two printf differ. If there 
is no outer `map(x)`, two printf should be identical.


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

https://reviews.llvm.org/D133694

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

[PATCH] D133325: [Driver] Allow search of included response files as configuration files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff marked 4 inline comments as done.
sepavloff added a comment.

In D133325#3771275 , @MaskRay wrote:

> For a config file, `--search-config-dirs` is introduced to change how `@cfg` 
> is resolved
> (relative to `--config-system-dir` or `--config-user-dir=`).
> I think mentioning the option names in the summary and having an elaborated 
> example in `clang/docs/UsersManual.rst` will help.

Added simple example in `clang/docs/UsersManual.rst`.

>> This change implements option --search-config-dirs, which modifies the 
>> search algorithm, - files included by @file are searched for as 
>> configuration files, in the same directories, and can be overridden by a 
>> user.
>
> Is the previous search algorithm sensible? If we think not, we can change the 
> default, and introduce a driver or CC1 option to stay the old behavior.
> My reasoning is that I cannot imagine many cases which can be broken by 
> changing the search algorithm.

There is nothing wrong with the previous algorithm, it is just for a different 
use case. For example, some users prepare set of config files, which contain 
set of options for various tasks, and put them to directories, which are easy 
to access (like `~/p1/dbg.cfg`). There files may share bulky information like 
include paths by using `@file` directives. Another directory may contain set of 
configuration files, say, for different platform. Enforcing the new algorithm 
would break existing work environments and require additional efforts to 
restore functionality.

One of the use case for the new algorithm is described in the patch 
description. As the option `--search-config-dirs` may be specified inside 
config file, from user perspective nothing changes, they just specify config 
file with the option `--config`, as previously. Compatibility is not broken.

> I think I'm still confused why we need a new option for changing the search 
> algorithm.
>
> Say, we have `--config-system-dir=system --config-user-dir=user` and three 
> files `system/a.cfg`, `system/b.cfg`, `user/b.cfg`.
> `system/a.cfg` contains `@b`.
> The old algorithm resolves to `system/b.cfg` (relative to the dir of 
> `system/a.cfg`: `system/`) while the new algorithm resolves to `user/b.cfg`?

You are right. It simplify preparing working environment by combining its parts 
(include paths, libraries, preprocessor definitions etc) taken from well-knows 
place, still allowing a user to modify it without need to copy all environment 
into a new directory and then changing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133325

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

[PATCH] D133659: [Clang] P1169R4: static operator()

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson marked 6 inline comments as done.
royjacobson added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:1265
+  if (Intro.hasLambdaCapture())
+P.Diag(StaticLoc, diag::err_static_lambda_captures);
+}

cor3ntin wrote:
> We might want to add a note showing where the capture list is.
I added a note, but the captures list is always just before the static 
specifier, so I'm not sure how useful it is. WDYT?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133659

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

[PATCH] D133659: [Clang] P1169R4: static operator()

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 459525.
royjacobson marked an inline comment as done.
royjacobson added a comment.

Add note in diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133659

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OperatorKinds.def
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/CXX/over/over.match/over.match.best/over.best.ics/p6.cpp
  clang/test/CXX/over/over.oper/p7.cpp
  clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
  clang/test/Parser/cxx2b-lambdas-ext-warns.cpp
  clang/test/Parser/cxx2b-lambdas.cpp
  clang/test/SemaCXX/overloaded-operator-decl.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1481,7 +1481,7 @@
 
   static operator()
   https://wg21.link/P1169R4";>P1169R4
-  No
+  Clang 16
 
 
   Extended floating-point types and standard names
Index: clang/test/SemaCXX/overloaded-operator-decl.cpp
===
--- clang/test/SemaCXX/overloaded-operator-decl.cpp
+++ clang/test/SemaCXX/overloaded-operator-decl.cpp
@@ -51,7 +51,7 @@
 
 namespace PR14120 {
   struct A {
-static void operator()(int& i) { ++i; } // expected-error{{overloaded 'operator()' cannot be a static member function}}
+static void operator()(int& i) { ++i; } // expected-error{{overloaded 'operator()' cannot be declared static}}
   };
   void f() {
 int i = 0;
Index: clang/test/Parser/cxx2b-lambdas.cpp
===
--- clang/test/Parser/cxx2b-lambdas.cpp
+++ clang/test/Parser/cxx2b-lambdas.cpp
@@ -38,3 +38,16 @@
 auto XL4 = [] requires true {}; // expected-error{{expected body}}
 auto XL5 = [] requires true requires true {}; // expected-error{{expected body}}
 auto XL6 = [] requires true noexcept requires true {}; // expected-error{{expected body}}
+
+auto XL7 = []() static static {}; // expected-error {{cannot appear multiple times}}
+auto XL8 = []() static mutable {}; // expected-error {{cannot be both mutable and static}}
+
+auto XL9 = [] static {};
+auto XL10 = []() static {};
+
+void static_captures() {
+  int x;
+  auto SC1 = [&]() static {}; // expected-error {{a static lambda cannot have any captures}} // expected-note {{captures declared here}}
+  auto SC2 = [&x]() static {}; // expected-error {{a static lambda cannot have any captures}} // expected-note {{captures declared here}}
+  auto SC3 = [=]() static {}; // expected-error {{a static lambda cannot have any captures}} // expected-note {{captures declared here}}
+}
Index: clang/test/Parser/cxx2b-lambdas-ext-warns.cpp
===
--- /dev/null
+++ clang/test/Parser/cxx2b-lambdas-ext-warns.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify=expected,cxx20
+// RUN: %clang_cc1 -std=c++2b %s -verify=expected,cxx2b
+
+//cxx2b-no-diagnostics
+
+auto L1 = [] constexpr {}; // cxx20-warning {{lambda without a parameter clause is a C++2b extension}}
+auto L2 = []() static {}; // cxx20-error {{static lambdas is a C++2b extension}}
+auto L3 = [] static {}; // cxx20-error {{static lambdas is a C++2b extension}} // cxx20-warning {{lambda without a parameter clause is a C++2b extension}}
Index: clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
@@ -0,0 +1,129 @@
+// RUN: %clang_cc1 -std=c++2b %s -emit-llvm -triple x86_64-linux -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++2b %s -emit-llvm -triple x86_64-windows-msvc -o - | FileCheck %s
+
+struct Functor {
+  static int operator()(int x, int y) {
+return x + y;
+  }
+};
+
+auto GetALambda() {
+  return [](int x, int y) static {
+return x + y;
+  };
+}
+
+void CallsTheLambda() {
+  GetALambda()(1, 2);
+}
+
+// CHECK:  define {{.*}}CallsTheLambda{{.*}}
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %call = call noundef i32 {{.*}}(i32 noundef 1, i32 noundef 2)
+// CHECK-NEXT:   ret void
+// CHECK-NEXT: }
+
+void call_static_call_operator() {
+  Functor f;
+  f(101, 102);
+  f.operator()(201, 202);
+  Functor{}(301, 302);
+}
+
+// CHECK:  define {{.*}}call_static_call_operator{{.*}}
+// CHECK-NEXT: entry:
+// CHECK:{{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 101, i32 noundef 102)
+// CHE

[PATCH] D133694: [Clang][OpenMP] Fix use_device_addr

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp:14
+{
+#pragma omp target data use_device_addr(x)
+{

ye-luo wrote:
> doru1004 wrote:
> > ye-luo wrote:
> > > In my understanding of the spec.
> > > `map(tofrom:x[0:256])` only maps the memory segment that x points to. x 
> > > itself as a pointer scalar is not mapped.
> > > use_device_addr(x) should fail to find the map of x scalar.
> > > 5.2 spec.
> > > If the list item is not a mapped list item, it is assumed to be 
> > > accessible on the target device.
> > > To me, it seems just keep &x as it was, in this case &x remains a host 
> > > address.
> > > 
> > > But in your patch description, it seems treating x differently from a 
> > > scalar.
> > > 
> > > I also applied your patch on main and got segfault because the x has a 
> > > value of device address and x[0] fails. This should be the behavior of 
> > > use_device_ptr instead of use_device_addr.
> > > To me, it seems just keep &x as it was, in this case &x remains a host 
> > > address.
> > 
> > So does this mean that if I do something like this in the target data I 
> > should get different addresses for x:
> > 
> > 
> > ```
> > #pragma omp target data use_device_ptr(x)
> > {
> > fprintf(stderr, "x: %p\n", __LINE__, x);
> > }
> > 
> > #pragma omp target data use_device_addr(x)
> > {
> > fprintf(stderr, "x: %p\n", __LINE__, x);
> > }
> > ```
> > 
> > 
> > > I also applied your patch on main and got segfault because the x has a 
> > > value of device address and x[0] fails.
> > 
> > That's my fault x[0] was the wrong thing to use actually.
> > 
> > 
> When you have an outer `target data map(x)`, then two printf differ. If there 
> is no outer `map(x)`, two printf should be identical.
> When you have an outer `target data map(x)`, then two printf differ. If there 
> is no outer `map(x)`, two printf should be identical.

This is super helpful thank you! I'll make sure that happens.

In the case when an outer target data exists, the print of the x which is under 
use_device_addr should print the same address as printing x on the host? 


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

https://reviews.llvm.org/D133694

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

[PATCH] D133694: [Clang][OpenMP] Fix use_device_addr

[Ye Luo via Phabricator via cfe-commits]

ye-luo added inline comments.



Comment at: clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp:14
+{
+#pragma omp target data use_device_addr(x)
+{

doru1004 wrote:
> ye-luo wrote:
> > doru1004 wrote:
> > > ye-luo wrote:
> > > > In my understanding of the spec.
> > > > `map(tofrom:x[0:256])` only maps the memory segment that x points to. x 
> > > > itself as a pointer scalar is not mapped.
> > > > use_device_addr(x) should fail to find the map of x scalar.
> > > > 5.2 spec.
> > > > If the list item is not a mapped list item, it is assumed to be 
> > > > accessible on the target device.
> > > > To me, it seems just keep &x as it was, in this case &x remains a host 
> > > > address.
> > > > 
> > > > But in your patch description, it seems treating x differently from a 
> > > > scalar.
> > > > 
> > > > I also applied your patch on main and got segfault because the x has a 
> > > > value of device address and x[0] fails. This should be the behavior of 
> > > > use_device_ptr instead of use_device_addr.
> > > > To me, it seems just keep &x as it was, in this case &x remains a host 
> > > > address.
> > > 
> > > So does this mean that if I do something like this in the target data I 
> > > should get different addresses for x:
> > > 
> > > 
> > > ```
> > > #pragma omp target data use_device_ptr(x)
> > > {
> > > fprintf(stderr, "x: %p\n", __LINE__, x);
> > > }
> > > 
> > > #pragma omp target data use_device_addr(x)
> > > {
> > > fprintf(stderr, "x: %p\n", __LINE__, x);
> > > }
> > > ```
> > > 
> > > 
> > > > I also applied your patch on main and got segfault because the x has a 
> > > > value of device address and x[0] fails.
> > > 
> > > That's my fault x[0] was the wrong thing to use actually.
> > > 
> > > 
> > When you have an outer `target data map(x)`, then two printf differ. If 
> > there is no outer `map(x)`, two printf should be identical.
> > When you have an outer `target data map(x)`, then two printf differ. If 
> > there is no outer `map(x)`, two printf should be identical.
> 
> This is super helpful thank you! I'll make sure that happens.
> 
> In the case when an outer target data exists, the print of the x which is 
> under use_device_addr should print the same address as printing x on the 
> host? 
I need a correction. When outer map(x) exists, actually the address(not value) 
of x should be a device address, and the code cannot even print x. Printing &x 
should be fine.


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

https://reviews.llvm.org/D133694

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

[PATCH] D133659: [Clang] P1169R4: static operator()

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added inline comments.



Comment at: clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp:5
+struct Functor {
+  static int operator()(int x, int y) {
+return x + y;

I want to see some tests that diagnose `extern operator()`.



Comment at: clang/test/Parser/cxx2b-lambdas.cpp:43
+auto XL7 = []() static static {}; // expected-error {{cannot appear multiple 
times}}
+auto XL8 = []() static mutable {}; // expected-error {{cannot be both mutable 
and static}}
+

And some tests that diagnose `extern`, `mutable extern`, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133659

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

[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Could you explain why this is necessary and even correct? I'd expect Clang to 
give an error when seeing `##` in this position, and I'd expect the scanner to 
do the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133674

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

[PATCH] D133705: [HIP] Fix unbundling archive

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

> Archives passed by -l: should not be prefixed with
>  prefix lib and appended with '.a', but still need to be prefixed with
>  paths in -L options.
>  Archives passed as input files should not be prefixed
> or appended with anything.

I'm not sure I understand the requirements. WDYM by "archives passed as input 
files should not be prefixed or appended by anything" ? E.g. if i do `clang -o 
foo foo.o mylib.a`, is `mylib.a` prefixed with something or has something 
appended?

It may help if you could rephrase it in terms of what *does* the code expect. 
E.g. 'archives with names `lib*.a` passed as direct inputs are not unbundled. 
All others are treated as potentially containing device code and are unbundled 
during [linking?]'.

Also, using `lib*.a` as pattern to tell device libraries from the host-ony one 
will be insufficient. There will be libraries with other extensions (e.g. `.lo` 
is fairly common) and there will be libraries that do not have `lib` prefix. 
I.e. nothing stops me from building `mylib.a` and linking with it as in the 
example above.


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

https://reviews.llvm.org/D133705

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

[PATCH] D133668: [HLSL] Use _BitInt(16) for int16_t to avoid promote to int.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133668#3783975 , @beanz wrote:

> In D133668#3783489 , @aaron.ballman 
> wrote:
>
>> Okay, that's good to know! If you don't intend to *ever* conform to the 
>> standard in this area, then I think this approach is very reasonable. But 
>> you should know up front that you're constraining yourself here. (Changing 
>> the underlying type in the future is an ABI break: 
>> https://godbolt.org/z/P6ndrzMab, note the name mangling.)
>
> We have the benefit of ABI escape hatches. HLSL itself doesn't define a 
> permanently stable ABI since GPU hardware and runtime ABIs change too 
> frequently. We instead revision our ABI every few years as the DirectX and 
> Vulkan specifications evolve.
>
> My hope is that as the HLSL language and our runtime ABIs evolve we'll be 
> more and more conformant to the C standard, but there are some odd areas that 
> we might never quite get there on.
>
> The 16-bit integer math is an interesting case. Because GPUs are inherently 
> SIMD machines, on many architectures you can handle twice as many 16-bit 
> operations per instruction as 32-bit (yay vectors!). Combine that with HLSL's 
> SPMD programming model and all scalar math is actually vector math. This 
> makes integer promotion for 16-bit types severely limiting. As a result I 
> don't suspect we'll ever want to conform to C here.

Ah, good to know!

Btw, it looks like precommit CI is finding failures here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133668

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

[clang] 3c1b423 - [clang] sort additional module maps when serializing

[Richard Howell via cfe-commits]

Author: Richard Howell
Date: 2022-09-12T12:00:43-07:00
New Revision: 3c1b42347b3a0666c93948ade2f420a20e060c1a

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

LOG: [clang] sort additional module maps when serializing

Sort additional module maps when serializing pcm files. This ensures
the `MODULE_MAP_FILE` record is deterministic across repeated builds.

Reviewed By: benlangmuir

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

Added: 


Modified: 
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 1e835a4c68531..838c6e306cfb0 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1282,7 +1282,12 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, 
ASTContext &Context,
 if (auto *AdditionalModMaps =
 Map.getAdditionalModuleMapFiles(WritingModule)) {
   Record.push_back(AdditionalModMaps->size());
-  for (const FileEntry *F : *AdditionalModMaps)
+  SmallVector ModMaps(AdditionalModMaps->begin(),
+AdditionalModMaps->end());
+  llvm::sort(ModMaps, [](const FileEntry *A, const FileEntry *B) {
+return A->getName() < B->getName();
+  });
+  for (const FileEntry *F : ModMaps)
 AddPath(F->getName(), Record);
 } else {
   Record.push_back(0);



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

[PATCH] D133611: [clang] sort additional module maps when serializing

[Richard Howell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c1b42347b3a: [clang] sort additional module maps when 
serializing (authored by rmaz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133611

Files:
  clang/lib/Serialization/ASTWriter.cpp


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1282,7 +1282,12 @@
 if (auto *AdditionalModMaps =
 Map.getAdditionalModuleMapFiles(WritingModule)) {
   Record.push_back(AdditionalModMaps->size());
-  for (const FileEntry *F : *AdditionalModMaps)
+  SmallVector ModMaps(AdditionalModMaps->begin(),
+AdditionalModMaps->end());
+  llvm::sort(ModMaps, [](const FileEntry *A, const FileEntry *B) {
+return A->getName() < B->getName();
+  });
+  for (const FileEntry *F : ModMaps)
 AddPath(F->getName(), Record);
 } else {
   Record.push_back(0);


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1282,7 +1282,12 @@
 if (auto *AdditionalModMaps =
 Map.getAdditionalModuleMapFiles(WritingModule)) {
   Record.push_back(AdditionalModMaps->size());
-  for (const FileEntry *F : *AdditionalModMaps)
+  SmallVector ModMaps(AdditionalModMaps->begin(),
+AdditionalModMaps->end());
+  llvm::sort(ModMaps, [](const FileEntry *A, const FileEntry *B) {
+return A->getName() < B->getName();
+  });
+  for (const FileEntry *F : ModMaps)
 AddPath(F->getName(), Record);
 } else {
   Record.push_back(0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D111283#3783338 , @mizvekov wrote:

> In D111283#3783250 , @ychen wrote:
>
>> Hi @mizvekov , I noticed that deduction for partial ordering also inherits 
>> this new behavior. Do you think partial ordering could opt out of this, or 
>> is it better for partial ordering to deal with the new sugared types?
>
> I would expect partial ordering to be performed only using canonical types, 
> that type sugar should make no difference there.
>
> Note that previous to this patch, we would already produce sugar on 
> deduction, but the behavior with regards to deducing the same template 
> parameter from multiple places was pretty badly behaved: We would just keep 
> the sugar from the first deduction, and ignore any subsequent ones. That 
> meant that the deduction order had an effect on the result. With this patch, 
> deduction becomes order agnostic.

I see. I  think it is definitely a good thing. I'm still learning what the 
expected AST should look like during the partial ordering.

> What kind of difference are you seeing?

For

  template  struct A {};
  
  template 
  bool foo(A);
  
  template 
  bool foo(A);
  
  template  bool bar()
  {
  return foo(Tuple{});
  }

`A` is currently modeled as ElaboratedType. It was 
`TemplateSpecializationType` before. Reading comments for `ElaboratedType`, it 
looks like sugar type might not be needed here?

  ElaboratedType 0xd79c8f0 'A' sugar dependent
  `-TemplateSpecializationType 0xd79c8b0 'A' dependent A
`-TemplateArgument type 'T'
  `-TemplateTypeParmType 0xd79c7f0 'T' dependent depth 0 index 0
`-TemplateTypeParm 0xd79c768 'T'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

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

Re: [clang] b7a7aee - [clang] Qualify auto in range-based for loops (NFC)

[Aaron Ballman via cfe-commits]

On Mon, Sep 12, 2022 at 12:17 PM David Blaikie  wrote:
>
> On Sat, Sep 10, 2022 at 3:01 PM Kazu Hirata  wrote:
> >
> > Thank you Aaron and David for your inputs.
> >
> > First and foremost, I apologize if I made your job harder by increasing the 
> > number of commits you have to peel to get to the real author.
> >
> > I hear that we are moving toward github pull requests.  A casual search 
> > tells me that there are some add-ons to integrate clang-tidy into the code 
> > review platform, so I am hoping we can use something like that to get each 
> > patch right first time.
> >
> > Going forward, I'll take git churn and the difficulty of backsliding as big 
> > factors in doing future clenaups.  For example, it's probably a good idea 
> > to delete a function that hasn't been used for many years (excluding dump 
> > functions and such).  Library standardization (like the recent removal of 
> > llvm::array_lengthof in favor of std::size) is less good in terms of git 
> > churn, but it's very unlikely for somebody to re-introduce 
> > llvm::array_lengthof.
>
> I think API deprecations (where the API can be completely removed
> eventually, and marked as deprecated/all in-tree usage removed within
> O(weeks/months)) especially for cases like the core/support libraries
> with relatively many uses, and relatively small APIs are great - if we
> get into the territory of naming convention cleanup, that gets more
> debatable because there's wide APIs with many naming violations and
> then we need more community discussion about what direction we're
> going (there are multiple lingering naming conventions, some
> discussions about moving to different ones in the future, so maybe
> churn to meet the current letter of the style guide would be better
> spent after changing the style guide with those directions in mind,
> etc).

+1

> For stylistic things like range-based-for conversion, const auto*, etc
> - yeah, there's some wriggle room depending on how uncontentious the
> conversion is, I think.

Agreed.

> (there's also some way to mark certain changes as ignorable by git?
> Maybe using that more frequently would help lower the cost of these
> sort of changes - broader discussion on discourse about ways to
> enable/lower the cost of these sort of changes would probably be good
> - I think as much as we can make these sort of changes cheaper/less
> problematic, to make them more encouraged, is a really good thing to
> do)

Strong +1! I want to find a way to encourage cleanups as much as we
reasonably can. I think using something like this:
https://akrabat.com/ignoring-revisions-with-git-blame/ is probably
worth exploring.

~Aaron

>
> >
> > Thanks,
> >
> > Kazu Hirata
> >
> >
> > On Fri, Sep 9, 2022 at 5:27 AM Aaron Ballman  wrote:
> >>
> >> On Thu, Sep 8, 2022 at 12:37 PM David Blaikie  wrote:
> >> >
> >> > Mixed feelings here - Kazu's made a lot of cleanup/stylistic changes
> >> > across the LLVM project for a while now, most, at least I think, are
> >> > quite welcome (things like switching to range-based-for, std
> >> > algorithms over llvm ones, llvm algorithms over manually written
> >> > loops, etc). But yeah, there's some threshold below which the churn
> >> > might not be worth the benefit - especially if the change doesn't come
> >> > along with tooling to enforce the invariant is maintained in the
> >> > future (if it's easy to make mistakes like this one - we'll regress it
> >> > and need to do cleanup again in the future)
> >>
> >> Thanks for speaking up, because I also waffled a bit on whether I
> >> called this out or not. :-)
> >>
> >> > Also, for this particular one, I wonder if in some cases this sort of
> >> > automatic transformation isn't ideal - if something is a pointer, but
> >> > that's an implementation detail, rather than an intentional feature of
> >> > an API (eg: the pointer-ness might be hidden behind a typedef and used
> >> > as an opaque handle, without any dereferencing, etc)
> >>
> >> Agreed.
> >>
> >> > I think it'd be really good to have some discussion on discourse about
> >> > if/how some of these cleanups could be formed into a way to
> >> > enforce/encourage the invariant to be maintained going forward -
> >> > clang-tidy (assuming that's the basis for the tooling Kazu's using to
> >> > make these changes in the first place) integration into the LLVM build
> >> > in some way, etc.
> >>
> >> I think that's a good idea! We want to encourage cleanups, but we
> >> don't want to encourage churn, and I think it's somewhat subjective
> >> where to draw that line. Having some more community awareness around
> >> that would be beneficial. I'm especially interested in how we balance
> >> between making incremental style improvements to the project and
> >> keeping our git blame logs useful. I'm seeing a lot more git blames
> >> that require several steps to get to an interesting commit because of
> >> the number of NFCs and reverts/recommits. Unfortunately, the tooling
> >> around vie

[PATCH] D133668: [HLSL] Use _BitInt(16) for int16_t to avoid promote to int.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added a comment.

In D133668#3784607 , @aaron.ballman 
wrote:

> In D133668#3783975 , @beanz wrote:
>
>> In D133668#3783489 , 
>> @aaron.ballman wrote:
>>
>>> Okay, that's good to know! If you don't intend to *ever* conform to the 
>>> standard in this area, then I think this approach is very reasonable. But 
>>> you should know up front that you're constraining yourself here. (Changing 
>>> the underlying type in the future is an ABI break: 
>>> https://godbolt.org/z/P6ndrzMab, note the name mangling.)
>>
>> We have the benefit of ABI escape hatches. HLSL itself doesn't define a 
>> permanently stable ABI since GPU hardware and runtime ABIs change too 
>> frequently. We instead revision our ABI every few years as the DirectX and 
>> Vulkan specifications evolve.
>>
>> My hope is that as the HLSL language and our runtime ABIs evolve we'll be 
>> more and more conformant to the C standard, but there are some odd areas 
>> that we might never quite get there on.
>>
>> The 16-bit integer math is an interesting case. Because GPUs are inherently 
>> SIMD machines, on many architectures you can handle twice as many 16-bit 
>> operations per instruction as 32-bit (yay vectors!). Combine that with 
>> HLSL's SPMD programming model and all scalar math is actually vector math. 
>> This makes integer promotion for 16-bit types severely limiting. As a result 
>> I don't suspect we'll ever want to conform to C here.
>
> Ah, good to know!
>
> Btw, it looks like precommit CI is finding failures here.

It is strange that the tests passed locally when CI hit fail in this PR. But in 
https://reviews.llvm.org/D133634, I hit fail locally while CI passed all tests.
I'll check and fix local failures I can repro first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133668

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

[PATCH] D133668: [HLSL] Use _BitInt(16) for int16_t to avoid promote to int.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added a comment.

In D133668#3784636 , @python3kgae 
wrote:

> In D133668#3784607 , @aaron.ballman 
> wrote:
>
>> In D133668#3783975 , @beanz wrote:
>>
>>> In D133668#3783489 , 
>>> @aaron.ballman wrote:
>>>
 Okay, that's good to know! If you don't intend to *ever* conform to the 
 standard in this area, then I think this approach is very reasonable. But 
 you should know up front that you're constraining yourself here. (Changing 
 the underlying type in the future is an ABI break: 
 https://godbolt.org/z/P6ndrzMab, note the name mangling.)
>>>
>>> We have the benefit of ABI escape hatches. HLSL itself doesn't define a 
>>> permanently stable ABI since GPU hardware and runtime ABIs change too 
>>> frequently. We instead revision our ABI every few years as the DirectX and 
>>> Vulkan specifications evolve.
>>>
>>> My hope is that as the HLSL language and our runtime ABIs evolve we'll be 
>>> more and more conformant to the C standard, but there are some odd areas 
>>> that we might never quite get there on.
>>>
>>> The 16-bit integer math is an interesting case. Because GPUs are inherently 
>>> SIMD machines, on many architectures you can handle twice as many 16-bit 
>>> operations per instruction as 32-bit (yay vectors!). Combine that with 
>>> HLSL's SPMD programming model and all scalar math is actually vector math. 
>>> This makes integer promotion for 16-bit types severely limiting. As a 
>>> result I don't suspect we'll ever want to conform to C here.
>>
>> Ah, good to know!
>>
>> Btw, it looks like precommit CI is finding failures here.
>
> It is strange that the tests passed locally when CI hit fail in this PR. But 
> in https://reviews.llvm.org/D133634, I hit fail locally while CI passed all 
> tests.
> I'll check and fix local failures I can repro first.

Might be some issue with the stack PR. Should be OK once rebase with 
https://reviews.llvm.org/D133634.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133668

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

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D111283#3784615 , @ychen wrote:

> `A` is currently modeled as ElaboratedType. It was 
> `TemplateSpecializationType` before. Reading comments for `ElaboratedType`, 
> it looks like sugar type might not be needed here?

Ah you might be seeing an ElaboratedTYpe here, where there was none before, 
perhaps because of https://reviews.llvm.org/D112374, and not because of this 
patch.

Yeah ElaboratedType is just sugar that should have no effect on partial 
ordering. But then I think no sugar should have effect on partial ordering. 
What is stopping you from just looking at the canonical type instead? On a 
canonical type, you would never see an ElaboratedType node, or a 
TemplateSpecializationType which is not dependent.

Is this related to the AutoType canonicalization issues we were discussing in 
the other patch of yours?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

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

[PATCH] D128958: Add assembler plumbing for sanitize_memtag

[Mitch Phillips via Phabricator via cfe-commits]

hctim updated this revision to Diff 459534.
hctim added a comment.

Remove unnecessary hasSanitizerMetadata()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128958

Files:
  clang/test/Driver/memtag-stack.c
  clang/test/Driver/memtag-stack_lto.c
  clang/test/Driver/memtag_lto.c
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/include/llvm/MC/MCAsmInfo.h
  llvm/include/llvm/MC/MCDirectives.h
  llvm/include/llvm/MC/MCELFObjectWriter.h
  llvm/include/llvm/MC/MCSymbolELF.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/MC/MCAsmStreamer.cpp
  llvm/lib/MC/MCELFStreamer.cpp
  llvm/lib/MC/MCMachOStreamer.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/MC/MCSymbolELF.cpp
  llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
  llvm/test/MC/AArch64/global-tagging.ll

Index: llvm/test/MC/AArch64/global-tagging.ll
===
--- /dev/null
+++ llvm/test/MC/AArch64/global-tagging.ll
@@ -0,0 +1,46 @@
+;; Tagged symbols are only available on aarch64-linux-android.
+; RUN: not llc %s -mtriple=aarch64-linux-unknown
+; RUN: not llc %s -mtriple=x86_64-linux-unknown
+
+; RUN: llc %s -mtriple=aarch64-linux-android31 -o %t.S
+; RUN: FileCheck %s --input-file=%t.S --check-prefix=CHECK-ASM
+; RUN: llvm-mc -filetype=obj %t.S -triple=aarch64-linux-android31 -o %t.o
+; RUN: llvm-readelf -r %t.o | FileCheck %s --check-prefix=CHECK-RELOCS
+
+; RUN: obj2yaml %t.o -o %t.yaml
+; RUN: FileCheck %s --input-file=%t.yaml --check-prefix=CHECK-YAML
+; RUN: yaml2obj %t.yaml -o %t.o
+; RUN: llvm-readelf -r %t.o | FileCheck %s --check-prefix=CHECK-RELOCS
+
+; CHECK-RELOCS: Relocation section '.rela.memtag.globals.static' {{.*}} contains 4 entries
+; CHECK-RELOCS: R_AARCH64_NONE {{.*}} internal_four
+; CHECK-RELOCS: R_AARCH64_NONE {{.*}} four
+; CHECK-RELOCS: R_AARCH64_NONE {{.*}} sixteen
+; CHECK-RELOCS: R_AARCH64_NONE {{.*}} huge
+; CHECK-RELOCS-NOT: specialcaselisted
+
+; CHECK-YAML:  Sections:
+; CHECK-YAML:  - Name: .rela.memtag.globals.static
+; CHECK-YAML-NOT:  - Name:
+; CHECK-YAML:  Relocations:
+; CHECK-YAML-NEXT: - Symbol: internal_four
+; CHECK-YAML-NEXT: Type: R_AARCH64_NONE
+; CHECK-YAML-NEXT: - Symbol: four
+; CHECK-YAML-NEXT: Type: R_AARCH64_NONE
+; CHECK-YAML-NEXT: - Symbol: sixteen
+; CHECK-YAML-NEXT: Type: R_AARCH64_NONE
+; CHECK-YAML-NEXT: - Symbol: huge
+; CHECK-YAML-NEXT: Type: R_AARCH64_NONE
+; CHECK-YAML-NEXT: -
+
+; CHECK-ASM: .memtag internal_four
+; CHECK-ASM: .memtag four
+; CHECK-ASM: .memtag sixteen
+; CHECK-ASM: .memtag huge
+; CHECK-ASM-NOT: .memtag specialcaselisted
+
+@internal_four = internal global i32 1, sanitize_memtag
+@four = global i32 1, sanitize_memtag
+@sixteen = global [16 x i8] zeroinitializer, sanitize_memtag
+@huge = global [16777232 x i8] zeroinitializer, sanitize_memtag
+@specialcaselisted = global i16 2
Index: llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
===
--- llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
+++ llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
@@ -34,6 +34,8 @@
 
   ~AArch64ELFObjectWriter() override = default;
 
+  MCSectionELF *getMemtagRelocsSection(MCContext &Ctx) const override;
+
 protected:
   unsigned getRelocType(MCContext &Ctx, const MCValue &Target,
 const MCFixup &Fixup, bool IsPCRel) const override;
@@ -453,6 +455,12 @@
   llvm_unreachable("Unimplemented fixup -> relocation");
 }
 
+MCSectionELF *
+AArch64ELFObjectWriter::getMemtagRelocsSection(MCContext &Ctx) const {
+  return Ctx.getELFSection(".memtag.globals.static",
+   ELF::SHT_ANDROID_MEMTAG_GLOBALS_STATIC, 0);
+}
+
 std::unique_ptr
 llvm::createAArch64ELFObjectWriter(uint8_t OSABI, bool IsILP32) {
   return std::make_unique(OSABI, IsILP32);
Index: llvm/lib/MC/MCSymbolELF.cpp
===
--- llvm/lib/MC/MCSymbolELF.cpp
+++ llvm/lib/MC/MCSymbolELF.cpp
@@ -33,7 +33,10 @@
   ELF_WeakrefUsedInReloc_Shift = 11,
 
   // One bit.
-  ELF_BindingSet_Shift = 12
+  ELF_BindingSet_Shift = 12,
+
+  // One bit.
+  ELF_IsMemoryTagged_Shift = 13,
 };
 }
 
@@ -193,4 +196,16 @@
 bool MCSymbolELF::isBindingSet() const {
   return getFlags() & (0x1 << ELF_BindingSet_Shift);
 }
+
+bool MCSymbolELF::isMemtag() const {
+  return getFlags() & (0x1 << ELF_IsMemoryTagged_Shift);
+}
+
+void MCSymbolELF::setMemtag(bool Tagged) {
+  uint32_t OtherFlags = getFlags() & ~(1 << ELF_IsMemoryTagged_Shift);
+  if (Tagged)
+setFlags(OtherFlags | (1 << ELF_IsMemoryTagged_Shift));
+  else
+setFlags(OtherFlags);
+}
 }
Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D111283#3784648 , @mizvekov wrote:

> In D111283#3784615 , @ychen wrote:
>
>> `A` is currently modeled as ElaboratedType. It was 
>> `TemplateSpecializationType` before. Reading comments for `ElaboratedType`, 
>> it looks like sugar type might not be needed here?
>
> Ah you might be seeing an ElaboratedTYpe here, where there was none before, 
> perhaps because of https://reviews.llvm.org/D112374, and not because of this 
> patch.
>
> Yeah ElaboratedType is just sugar that should have no effect on partial 
> ordering. But then I think no sugar should have effect on partial ordering. 
> What is stopping you from just looking at the canonical type instead? On a 
> canonical type, you would never see an ElaboratedType node, or a 
> TemplateSpecializationType which is not dependent.

Thanks for the link. I'm not blocked by any of these patches, instead just 
trying to have a mental model of when to expect the sugared type :-). For 
partial ordering, the `TemplateSpecializationType` could be dependent, since 
the injected template arguments are dependent, I guess that's the reason there 
is the `ElaboratedType`?

> Is this related to the AutoType canonicalization issues we were discussing in 
> the other patch of yours?

Nope. I found this AST difference while investigating D133683 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

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

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D111283#3784663 , @ychen wrote:

> Thanks for the link. I'm not blocked by any of these patches, instead just 
> trying to have a mental model of when to expect the sugared type :-). For 
> partial ordering, the `TemplateSpecializationType` could be dependent, since 
> the injected template arguments are dependent, I guess that's the reason 
> there is the `ElaboratedType`?

The ElaboratedType is a sort of a `companion node` to other nodes that 
represent things in the language which can have (non-dependent) nested name 
qualifiers (a NNS for short) and / or an elaborated type specifier (such as the 
`struct` in `struct A`).

It's only purpose is to carry that extra bit of data, like some external 
storage really, and it shouldn't affect the semantics of the program once the 
source code is parsed into an AST.

Here, in your example, the ElaboratedType is there, as a companion to that 
TemplateSpecializationType, just to say that this template specialization was 
written without any name qualifiers nor elaboration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

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

[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

[Argyrios Kyrtzidis via Phabricator via cfe-commits]

akyrtzi added a comment.

In D133674#3784602 , @jansvoboda11 
wrote:

> Could you explain why this is necessary and even correct? I'd expect Clang to 
> give an error when seeing `##` in this position, and I'd expect the scanner 
> to do the same.

`##` is lexed as `tok::hashhash`; it is ignored by the preprocessor (it is not 
treated as the start of a preprocessor directive) and passed on to the parser 
to interpret, like any other token.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133674

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

[PATCH] D132975: [CMake] Add clang-bolt target

[Amir Ayupov via Phabricator via cfe-commits]

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

Address @phosek's comment about dependency on shell


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
  clang/utils/perf-training/perf-helper.py

Index: clang/utils/perf-training/perf-helper.py
===
--- clang/utils/perf-training/perf-helper.py
+++ clang/utils/perf-training/perf-helper.py
@@ -38,7 +38,7 @@
 
 def merge(args):
   if len(args) != 3:
-print('Usage: %s clean   \n' % __file__ +
+print('Usage: %s merge   \n' % __file__ +
   '\tMerges all profraw files from path into output.')
 return 1
   cmd = [args[0], 'merge', '-o', args[1]]
@@ -46,6 +46,16 @@
   subprocess.check_call(cmd)
   return 0
 
+def merge_fdata(args):
+  if len(args) != 3:
+print('Usage: %s merge-fdata   \n' % __file__ +
+  '\tMerges all fdata files from path into output.')
+return 1
+  cmd = [args[0], '-o', args[1]]
+  cmd.extend(findFilesWithExtension(args[2], "fdata"))
+  subprocess.check_call(cmd)
+  return 0
+
 def dtrace(args):
   parser = argparse.ArgumentParser(prog='perf-helper dtrace',
 description='dtrace wrapper for order file generation')
@@ -395,10 +405,12 @@
   return 0
 
 commands = {'clean' : clean,
-  'merge' : merge, 
+  'merge' : merge,
   'dtrace' : dtrace,
   'cc1' : cc1,
-  'gen-order-file' : genOrderFile}
+  'gen-order-file' : genOrderFile,
+  'merge-fdata' : merge_fdata,
+  }
 
 def main():
   f = commands[sys.argv[1]]
Index: clang/cmake/caches/BOLT.cmake
===
--- /dev/null
+++ clang/cmake/caches/BOLT.cmake
@@ -0,0 +1,14 @@
+set(CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(CLANG_BOLT_INSTRUMENT ON CACHE BOOL "")
+set(CLANG_BOLT_INSTRUMENT_PROJECTS "llvm" CACHE STRING "")
+set(CLANG_BOLT_INSTRUMENT_TARGETS "count" CACHE STRING "")
+set(CMAKE_EXE_LINKER_FLAGS "-Wl,--emit-relocs,-znow" CACHE STRING "")
+
+set(LLVM_ENABLE_PROJECTS "bolt;clang" 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
@@ -443,7 +443,7 @@
   "HAVE_CLANG_PLUGIN_SUPPORT" OFF)
 
 # If libstdc++ is statically linked, clang-repl needs to statically link libstdc++
-# itself, which is not possible in many platforms because of current limitations in 
+# itself, which is not possible in many platforms because of current limitations in
 # JIT stack. (more platforms need to be supported by JITLink)
 if(NOT LLVM_STATIC_LINK_CXX_STDLIB)
   set(HAVE_CLANG_REPL_SUPPORT ON)
@@ -881,6 +881,117 @@
   endforeach()
 endif()
 
+if (CLANG_BOLT_INSTRUMENT)
+  set(CLANG_PATH ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang)
+  set(CLANGXX_PATH ${CLANG_PATH}++)
+  set(CLANG_INSTRUMENTED ${CLANG_PATH}-bolt.inst)
+  set(CLANGXX_INSTRUMENTED ${CLANGXX_PATH}-bolt.inst)
+  set(CLANG_OPTIMIZED ${CLANG_PATH}-bolt)
+  set(CLANGXX_OPTIMIZED ${CLANGXX_PATH}-bolt)
+
+  # Instrument clang with BOLT
+  add_custom_target(clang-instrumented
+DEPENDS ${CLANG_INSTRUMENTED}
+  )
+  add_custom_command(OUTPUT ${CLANG_INSTRUMENTED}
+DEPENDS clang llvm-bolt
+COMMAND llvm-bolt ${CLANG_PATH} -o ${CLANG_INSTRUMENTED}
+  -instrument --instrumentation-file-append-pid
+  --instrumentation-file=${CMAKE_CURRENT_BINARY_DIR}/prof.fdata
+COMMENT "Instrumenting clang binary with BOLT"
+VERBATIM
+  )
+
+  # Make a symlink from clang-bolt.inst to clang++-bolt.inst
+  add_custom_target(clang++-instrumented
+DEPENDS ${CLANGXX_INSTRUMENTED}
+  )
+  add_custom_command(OUTPUT ${CLANGXX_INSTRUMENTED}
+DEPENDS clang-instrumented
+COMMAND ${CMAKE_COMMAND} -E create_symlink
+  ${CLANG_INSTRUMENTED}
+  ${CLANGXX_INSTRUMENTED}
+COMMENT "Creating symlink from BOLT instrumented clang to clang++"
+VERBATIM
+  )
+
+  # Build specified targets with instrumented Clang to collect the profile
+  set(STAMP_DIR ${CMAKE_CURRENT_BINARY_DIR}/bolt-instrumented-clang-stamps/)
+  set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/bolt-instrumented-clang-bins/)
+  set(build_configuration "$")
+  include(ExternalProject)
+  ExternalProject_Add(bolt-instrumentation-profile
+DEPENDS clang++-instrumented
+PREFIX bolt-instrumentation-profile
+SOURCE_DIR ${CMAKE_SOURCE_DIR}
+STAMP_DIR ${STAMP_DIR}
+BINARY_DIR ${BINARY_DIR}
+EXCLUDE_FROM_ALL 1
+CMAKE_ARGS
+# We shouldn't need to set this here, but INSTALL_DIR doesn't
+# seem to work, so instead

[PATCH] D132975: [CMake] Add clang-bolt target

[Amir Ayupov via Phabricator via cfe-commits]

Amir marked an inline comment as not done.
Amir added inline comments.



Comment at: clang/CMakeLists.txt:930-937
+-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
+-DCMAKE_C_COMPILER=${CLANG_INSTRUMENTED}
+-DCMAKE_CXX_COMPILER=${CLANGXX_INSTRUMENTED}
+-DCMAKE_ASM_COMPILER=${CLANG_INSTRUMENTED}
+-DCMAKE_ASM_COMPILER_ID=Clang
+-DCMAKE_BUILD_TYPE=Release
+-DLLVM_ENABLE_PROJECTS=${CLANG_BOLT_INSTRUMENT_PROJECTS}

MaskRay wrote:
> phosek wrote:
> > I don't think this is sufficient in the general case, we would need to pass 
> > additional variables like `CMAKE_AR` the same way we do for the existing 
> > bootstrap logic, see 
> > https://github.com/llvm/llvm-project/blob/dc549bf0013e11e8fcccba8a8d59c3a4bb052a3b/clang/CMakeLists.txt#L825.
> > 
> > For example, on Fuchsia builders we don't have any system-wide toolchain 
> > installation, instead we manually set  all necessary `CMAKE_` 
> > variables for the first stage, so this call will fail for us because it 
> > won't be able to find tools like the archiver.
> > 
> > Since handling this properly would likely duplicate a lot of the existing 
> > logic from the existing bootstrap logic, I'm wondering if we should instead 
> > try to refactor the existing logic and break it up into macros/functions 
> > which could then be reused here as well.
> Supporting other cmake variables will be awesome. I use something like 
> `-DCMAKE_CXX_ARCHIVE_CREATE="$HOME/llvm/out/stable/bin/llvm-ar qcS --thin 
>  " -DCMAKE_CXX_ARCHIVE_FINISH=:` to make my build smaller.
Addressed in D133633


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132975

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

[clang] 4a72459 - Revert "[clang][test] Disallow using the default module cache path in lit tests"

[Ben Langmuir via cfe-commits]

Author: Ben Langmuir
Date: 2022-09-12T13:10:22-07:00
New Revision: 4a72459ed639e3eb7188565c758181c43d3192aa

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

LOG: Revert "[clang][test] Disallow using the default module cache path in lit 
tests"

This reverts commit d96f526196ac4cebfdd318473816f6d4b9d76707.

Some systems do not support `env -u`.

Added: 


Modified: 
clang/test/Driver/modules-cache-path.m
clang/test/Modules/driver.c
llvm/utils/lit/lit/llvm/config.py

Removed: 




diff  --git a/clang/test/Driver/modules-cache-path.m 
b/clang/test/Driver/modules-cache-path.m
index f700a9738742f..1da27d2143631 100644
--- a/clang/test/Driver/modules-cache-path.m
+++ b/clang/test/Driver/modules-cache-path.m
@@ -1,4 +1,4 @@
-// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -### %s 2>&1 | 
FileCheck %s -check-prefix=CHECK-DEFAULT
+// RUN: %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 \

diff  --git a/clang/test/Modules/driver.c b/clang/test/Modules/driver.c
index 8ffa23ba4e71c..34fc163a5ccd4 100644
--- a/clang/test/Modules/driver.c
+++ b/clang/test/Modules/driver.c
@@ -1,4 +1,4 @@
-// RUN: env -u CLANG_MODULE_CACHE_PATH %clang -fmodules -fimplicit-module-maps 
%s -### 2>&1 | FileCheck -check-prefix CHECK-NO_MODULE_CACHE %s
+// RUN: %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"}}

diff  --git a/llvm/utils/lit/lit/llvm/config.py 
b/llvm/utils/lit/lit/llvm/config.py
index 6149f4db7e0ca..591b9938f211f 100644
--- a/llvm/utils/lit/lit/llvm/config.py
+++ b/llvm/utils/lit/lit/llvm/config.py
@@ -495,10 +495,6 @@ def use_clang(self, additional_tool_dirs=[], 
additional_flags=[],
 
 self.clear_environment(possibly_dangerous_env_vars)
 
-# Make the default module cache path invalid so that tests are forced 
to
-# provide a cache path if they use implicit modules.
-self.with_environment('CLANG_MODULE_CACHE_PATH', '/dev/null')
-
 # Tweak the PATH to include the tools dir and the scripts dir.
 # Put Clang first to avoid LLVM from overriding out-of-tree clang
 # builds.



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

[PATCH] D133574: [C2x] reject type definitions in offsetof

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/include/clang/Parse/Parser.h:2283
 case DeclSpecContext::DSC_top_level:
+case DeclSpecContext::DSC_offsetof:
   return true;

Why `true` for this case? What does this allow that we want? Do we test it?



Comment at: clang/lib/Parse/ParseExpr.cpp:2583
+TypeResult Ty =
+ParseTypeName(/*Range*/ nullptr, DeclaratorContext::OffsetOf);
 if (Ty.isInvalid()) {




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133574

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

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added a comment.

Reverted due to failure on a bot 
https://lab.llvm.org/buildbot/#/builders/214/builds/3252

I'm not sure how to deal with missing `env -u`.

- We could do `env CLANG_MODULE_CACHE_PATH=` and change the compiler's 
interpretation of empty string for this variable. I'm not sure if the current 
behaviour (there will be no module cache in the cc1 at all) is intentional or 
useful.  Hesitant to change this behaviour.
- We could try to enumerate all the environments that don't support `env -u` 
and disable these two tests on  those platforms.  So far it was just one AIX 
bot, but I wouldn't be surprised if other less commonly used unixes have the 
same issue.
- We could make the command inscrutable, like `not env -u X true || env -u ... 
real command ...` so that if `env -u X true` fails (presumably due to not 
supporting `-u` option) we won't run the rest of the RUN line.

If someone has a suggestion for a simple fix, I can try again.  But otherwise I 
doubt it's worth putting much time into this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133622

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

[PATCH] D133589: [clang-format] JSON formatting add new option for controlling newlines in json arrays

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

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/Format.cpp:1965
 (Style.JavaScriptQuotes == FormatStyle::JSQS_Double &&
- !Input.startswith("\'"))) {
+ !Input.startswith("\'")))
   continue;

These braces should stay, shouldn't they?



Comment at: clang/lib/Format/Format.cpp:2190
   Matching->is(TT_ArrayInitializerLSquare)) &&
-!(FormatTok->is(tok::r_brace) && Matching->is(TT_DictLiteral))) {
+!(FormatTok->is(tok::r_brace) && Matching->is(TT_DictLiteral)))
   continue;

It seems like there are a ton of unrelated brace removals.


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

https://reviews.llvm.org/D133589

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

[PATCH] D133571: [clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

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

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:469
+  /// Different styles for merging short case labels.
+  enum ShortCaseLabelStyle : int8_t {
+/// Never merge case code

MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > While we're at it, shouldn't there be a `Leave`? ;)
> I think Leave is Never
No. `Leave` would leave me with this:
```
switch (a) {
  case 1: x = 1; break;
  case 2:
return;
}
```
If I'd start with it. `Leave` allows to mix the Yes and No options.


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

https://reviews.llvm.org/D133571

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

[PATCH] D133648: Clang, increase upper bound of partially initialized array sizes

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:866
 ElementEntity.getKind() == InitializedEntity::EK_VectorElement)
   ElementEntity.setElementIndex(Init);
 

`setElementIndex(...)` takes `unsigned` as well and therefore 
`InitializedEntity` also uses `unsigned`.

I briefly looked at this with another bug 
https://github.com/llvm/llvm-project/issues/57317

and I believe the 32 bit assumption is made in a lot of this code in this area. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133648

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

[PATCH] D133725: Searching for tokens including comments

[Bartłomiej Cieślar via Phabricator via cfe-commits]

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

This is a small diff that adds an optional argument to the Lexer::findNextToken 
that allows for searching for the token while including comments (instead of 
having to 
instantiate the lexer separately)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133725

Files:
  clang/include/clang/Lex/Lexer.h
  clang/lib/Lex/Lexer.cpp
  clang/unittests/Lex/LexerTest.cpp


Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -622,6 +622,24 @@
 "xyz", "=", "abcd", ";"));
 }
 
+TEST_F(LexerTest, FindNextTokenWithComments) {
+  Lex("int /* type */ abcd = 0; // vardecl\n"
+  "int /*other type*/ xyz = abcd; //other vardecl \n");
+  std::vector GeneratedByNextToken;
+  SourceLocation Loc =
+  SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID());
+  while (true) {
+auto T = Lexer::findNextToken(Loc, SourceMgr, LangOpts, true);
+ASSERT_TRUE(T);
+if (T->is(tok::eof))
+  break;
+GeneratedByNextToken.push_back(getSourceText(*T, *T));
+Loc = T->getLocation();
+  }
+  EXPECT_THAT(GeneratedByNextToken, ElementsAre("/* type */", "abcd", "=", 
"0", ";", "// vardecl", "int", "/*other type*/",
+"xyz", "=", "abcd", ";", 
"//other vardecl "));
+}
+
 TEST_F(LexerTest, CreatedFIDCountForPredefinedBuffer) {
   TrivialModuleLoader ModLoader;
   auto PP = CreatePP("", ModLoader);
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -1258,7 +1258,8 @@
 
 Optional Lexer::findNextToken(SourceLocation Loc,
  const SourceManager &SM,
- const LangOptions &LangOpts) {
+ const LangOptions &LangOpts,
+ bool IncludeComments) {
   if (Loc.isMacroID()) {
 if (!Lexer::isAtEndOfMacroExpansion(Loc, SM, LangOpts, &Loc))
   return None;
@@ -1274,11 +1275,13 @@
   if (InvalidTemp)
 return None;
 
-  const char *TokenBegin = File.data() + LocInfo.second;
+  const char *const TokenBegin = File.data() + LocInfo.second;
 
   // Lex from the start of the given location.
   Lexer lexer(SM.getLocForStartOfFile(LocInfo.first), LangOpts, File.begin(),
   TokenBegin, File.end());
+  lexer.SetCommentRetentionState(IncludeComments);
+
   // Find the token.
   Token Tok;
   lexer.LexFromRawLexer(Tok);
Index: clang/include/clang/Lex/Lexer.h
===
--- clang/include/clang/Lex/Lexer.h
+++ clang/include/clang/Lex/Lexer.h
@@ -554,7 +554,8 @@
   /// Returns the next token, or none if the location is inside a macro.
   static Optional findNextToken(SourceLocation Loc,
const SourceManager &SM,
-   const LangOptions &LangOpts);
+   const LangOptions &LangOpts,
+   bool IncludeComments = false);
 
   /// Checks that the given token is the first token that occurs after
   /// the given location (this excludes comments and whitespace). Returns the


Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -622,6 +622,24 @@
 "xyz", "=", "abcd", ";"));
 }
 
+TEST_F(LexerTest, FindNextTokenWithComments) {
+  Lex("int /* type */ abcd = 0; // vardecl\n"
+  "int /*other type*/ xyz = abcd; //other vardecl \n");
+  std::vector GeneratedByNextToken;
+  SourceLocation Loc =
+  SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID());
+  while (true) {
+auto T = Lexer::findNextToken(Loc, SourceMgr, LangOpts, true);
+ASSERT_TRUE(T);
+if (T->is(tok::eof))
+  break;
+GeneratedByNextToken.push_back(getSourceText(*T, *T));
+Loc = T->getLocation();
+  }
+  EXPECT_THAT(GeneratedByNextToken, ElementsAre("/* type */", "abcd", "=", "0", ";", "// vardecl", "int", "/*other type*/",
+"xyz", "=", "abcd", ";", "//other vardecl "));
+}
+
 TEST_F(LexerTest, CreatedFIDCountForPredefinedBuffer) {
   TrivialModuleLoader ModLoader;
   auto PP = CreatePP("", ModLoader);
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -1258,7 +1258,8 @@
 
 Optional Lexer::findNextToken(SourceLocation Loc,

[PATCH] D133694: [Clang][OpenMP] Fix use_device_addr

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/test/OpenMP/target_data_use_device_addr_codegen_ptr.cpp:14
+{
+#pragma omp target data use_device_addr(x)
+{

ye-luo wrote:
> doru1004 wrote:
> > ye-luo wrote:
> > > doru1004 wrote:
> > > > ye-luo wrote:
> > > > > In my understanding of the spec.
> > > > > `map(tofrom:x[0:256])` only maps the memory segment that x points to. 
> > > > > x itself as a pointer scalar is not mapped.
> > > > > use_device_addr(x) should fail to find the map of x scalar.
> > > > > 5.2 spec.
> > > > > If the list item is not a mapped list item, it is assumed to be 
> > > > > accessible on the target device.
> > > > > To me, it seems just keep &x as it was, in this case &x remains a 
> > > > > host address.
> > > > > 
> > > > > But in your patch description, it seems treating x differently from a 
> > > > > scalar.
> > > > > 
> > > > > I also applied your patch on main and got segfault because the x has 
> > > > > a value of device address and x[0] fails. This should be the behavior 
> > > > > of use_device_ptr instead of use_device_addr.
> > > > > To me, it seems just keep &x as it was, in this case &x remains a 
> > > > > host address.
> > > > 
> > > > So does this mean that if I do something like this in the target data I 
> > > > should get different addresses for x:
> > > > 
> > > > 
> > > > ```
> > > > #pragma omp target data use_device_ptr(x)
> > > > {
> > > > fprintf(stderr, "x: %p\n", __LINE__, x);
> > > > }
> > > > 
> > > > #pragma omp target data use_device_addr(x)
> > > > {
> > > > fprintf(stderr, "x: %p\n", __LINE__, x);
> > > > }
> > > > ```
> > > > 
> > > > 
> > > > > I also applied your patch on main and got segfault because the x has 
> > > > > a value of device address and x[0] fails.
> > > > 
> > > > That's my fault x[0] was the wrong thing to use actually.
> > > > 
> > > > 
> > > When you have an outer `target data map(x)`, then two printf differ. If 
> > > there is no outer `map(x)`, two printf should be identical.
> > > When you have an outer `target data map(x)`, then two printf differ. If 
> > > there is no outer `map(x)`, two printf should be identical.
> > 
> > This is super helpful thank you! I'll make sure that happens.
> > 
> > In the case when an outer target data exists, the print of the x which is 
> > under use_device_addr should print the same address as printing x on the 
> > host? 
> I need a correction. When outer map(x) exists, actually the address(not 
> value) of x should be a device address, and the code cannot even print x. 
> Printing &x should be fine.
In the context of the above comment, should &x on the device be an address I 
can verify, somehow, to make sure that it's correct or is it a completely new 
device address?

So for example, should it be the same as when I do a use_device_ptr but print 
the &x in that case? (With the current master those two addresses are not the 
same.)

I guess what I need is an example of using use_device_addr that actually does 
something meaningful because with the current main branch printing the &x of a 
use_device_addr(x) is nil.


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

https://reviews.llvm.org/D133694

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

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D111283#3784702 , @mizvekov wrote:

> In D111283#3784663 , @ychen wrote:
>
>> Thanks for the link. I'm not blocked by any of these patches, instead just 
>> trying to have a mental model of when to expect the sugared type :-). For 
>> partial ordering, the `TemplateSpecializationType` could be dependent, since 
>> the injected template arguments are dependent, I guess that's the reason 
>> there is the `ElaboratedType`?
>
> The ElaboratedType is a sort of a `companion node` to other nodes that 
> represent things in the language which can have (non-dependent) nested name 
> qualifiers (a NNS for short) and / or an elaborated type specifier (such as 
> the `struct` in `struct A`).
>
> It's only purpose is to carry that extra bit of data, like some external 
> storage really, and it shouldn't affect the semantics of the program once the 
> source code is parsed into an AST.
>
> Here, in your example, the ElaboratedType is there, as a companion to that 
> TemplateSpecializationType, just to say that this template specialization was 
> written without any name qualifiers nor elaboration.

Very helpful explanation :-).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

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

[PATCH] D133648: Clang, increase upper bound of partially initialized array sizes

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:866
 ElementEntity.getKind() == InitializedEntity::EK_VectorElement)
   ElementEntity.setElementIndex(Init);
 

shafik wrote:
> `setElementIndex(...)` takes `unsigned` as well and therefore 
> `InitializedEntity` also uses `unsigned`.
> 
> I briefly looked at this with another bug 
> https://github.com/llvm/llvm-project/issues/57317
> 
> and I believe the 32 bit assumption is made in a lot of this code in this 
> area. 
You don't need to solve everything at once...



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:3167
   // Print the fields in successive locations. Pad to align if needed!
-  unsigned Size = DL.getTypeAllocSize(CS->getType());
+  uint64_t Size = DL.getTypeAllocSize(CS->getType());
   const StructLayout *Layout = DL.getStructLayout(CS->getType());

Can you split this into a separate patch (with an appropriate testcase)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133648

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

[clang] ab56719 - [clang, llvm] Add __declspec(safebuffers), support it in CodeView

[David Majnemer via cfe-commits]

Author: David Majnemer
Date: 2022-09-12T21:15:34Z
New Revision: ab56719acd98778fb2e48fa425ac7c8d27bdea86

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

LOG: [clang, llvm] Add __declspec(safebuffers), support it in CodeView

__declspec(safebuffers) is equivalent to
__attribute__((no_stack_protector)).  This information is recorded in
CodeView.

While we are here, add support for strict_gs_check.

Added: 


Modified: 
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/Parser/MicrosoftExtensions.c
llvm/include/llvm/BinaryFormat/COFF.h
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
llvm/lib/Target/X86/X86AsmPrinter.cpp
llvm/test/DebugInfo/COFF/frameproc-flags.ll
llvm/test/DebugInfo/COFF/multifunction.ll
llvm/test/DebugInfo/COFF/simple.ll
llvm/test/DebugInfo/COFF/vframe-fpo.ll

Removed: 




diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 0ad59b2c153ae..af0e086540146 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2080,12 +2080,19 @@ def NotTailCalled : InheritableAttr {
 def : MutualExclusions<[AlwaysInline, NotTailCalled]>;
 
 def NoStackProtector : InheritableAttr {
-  let Spellings = [Clang<"no_stack_protector">];
+  let Spellings = [Clang<"no_stack_protector">, Declspec<"safebuffers">];
   let Subjects = SubjectList<[Function]>;
   let Documentation = [NoStackProtectorDocs];
   let SimpleHandler = 1;
 }
 
+def StrictGuardStackCheck : InheritableAttr {
+  let Spellings = [Declspec<"strict_gs_check">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [StrictGuardStackCheckDocs];
+  let SimpleHandler = 1;
+}
+
 def NoThrow : InheritableAttr {
   let Spellings = [GCC<"nothrow">, Declspec<"nothrow">];
   let Subjects = SubjectList<[FunctionLike]>;

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 68f1268ed7da9..b6c15c92d5815 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -4453,7 +4453,8 @@ spelled "XYZ" in the `OpenMP 5.1 Standard`_).
 def NoStackProtectorDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
-Clang supports the ``__attribute__((no_stack_protector))`` attribute which 
disables
+Clang supports the ``__attribute__((no_stack_protector))`` and Microsoft style
+``__declspec(safebuffers)`` attribute which disables
 the stack protector on the specified function. This attribute is useful for
 selectively disabling the stack protector on some functions when building with
 ``-fstack-protector`` compiler option.
@@ -4472,6 +4473,27 @@ option.
 }];
 }
 
+def StrictGuardStackCheckDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+Clang supports the Microsoft style ``__declspec((strict_gs_check))`` attribute
+which upgrades the stack protector check from ``-fstack-protector`` to
+``-fstack-protector-strong``.
+
+For example, it upgrades the stack protector for the function ``foo`` to
+``-fstack-protector-strong`` but function ``bar`` will still be built with the
+stack protector with the ``-fstack-protector`` option.
+
+.. code-block:: c
+
+__declspec((strict_gs_check))
+int foo(int x); // stack protection will be upgraded for foo.
+
+int bar(int y); // bar can be built with the standard stack protector 
checks.
+
+}];
+}
+
 def NotTailCalledDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index f9087cdd5d4dc..ad6eb32ccc649 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1957,14 +1957,17 @@ void 
CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D,
   if (!hasUnwindExceptions(LangOpts))
 B.addAttribute(llvm::Attribute::NoUnwind);
 
-  if (!D || !D->hasAttr()) {
-if (LangOpts.getStackProtector() == LangOptions::SSPOn)
-  B.addAttribute(llvm::Attribute::StackProtect);
-else if (LangOpts.getStackProtector() == LangOptions::SSPStrong)
-  B.addAttribute(llvm::Attribute::StackProtectStrong);
-else if (LangOpts.getStackProtector() == LangOptions::SSPReq)
-  B.addAttribute(llvm::Attribute::StackProtectReq);
-  }
+  if (D && D->hasAttr())
+; // Do nothing.
+  else if (D && D->hasAttr() &&
+   LangOpts.getStackProtector() == LangOptions::SSPOn)
+B.addAttribute(llvm::Attribute::StackProtectStrong);
+  else if (LangOpts.getStackProtector() == LangOptions::SSPOn)
+B.addAttribute(llvm::Attribute::StackProtect);
+  else if (LangOpts.getStackProt

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, JonChesterfield, ronlieb, arsenm, yaxunl, 
tianshilei1992, ye-luo.
Herald added subscribers: kosarev, kerbowa, guansong, t-tye, tpr, dstuttard, 
jvesely, kzhuravl.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, MaskRay, wdng.
Herald added a project: clang.

Previously, we linked in the ROCm device libraries which provide math
and other utility functions late. This is not stricly correct as this
library contains several flags that are only set per-TU, such as fast
math or denormalization. This patch changes this to pass the bitcode
libraries per-TU using the same method we use for the CUDA libraries.
This has the advantage that we correctly propagate attributes making
this implementation more correct. Additionally, many annoying unused
functions were not being fully removed during LTO. This lead to
erroneous warning messages and remarks on unused functions.

I am not sure if not finding these libraries should be a hard error. let
me know if it should be demoted to a warning saying that some device
utilities will not work without them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133726

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/amdgpu-openmp-toolchain.c

Index: clang/test/Driver/amdgpu-openmp-toolchain.c
===
--- clang/test/Driver/amdgpu-openmp-toolchain.c
+++ clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -49,5 +49,7 @@
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-EMIT-LLVM-IR
 // CHECK-EMIT-LLVM-IR: "-cc1" "-triple" "amdgcn-amd-amdhsa"{{.*}}"-emit-llvm"
 
-// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 -lm --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode %s 2>&1 | FileCheck %s --check-prefix=CHECK-LIB-DEVICE-NEW
-// CHECK-LIB-DEVICE-NEW: {{.*}}clang-linker-wrapper{{.*}}--bitcode-library=openmp-amdgcn-amd-amdhsa-gfx803={{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
+// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp --offload-arch=gfx803 \
+// RUN:   --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode -fopenmp-new-driver %s  2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-LIB-DEVICE-NEW
+// CHECK-LIB-DEVICE-NEW: "-cc1" {{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8367,7 +8367,6 @@
  const char *LinkingOutput) const {
   const Driver &D = getToolChain().getDriver();
   const llvm::Triple TheTriple = getToolChain().getTriple();
-  auto OpenMPTCRange = C.getOffloadToolChains();
   ArgStringList CmdArgs;
 
   // Pass the CUDA path to the linker wrapper tool.
@@ -8385,30 +8384,6 @@
 }
   }
 
-  // Get the AMDGPU math libraries.
-  // FIXME: This method is bad, remove once AMDGPU has a proper math library
-  // (see AMDGCN::OpenMPLinker::constructLLVMLinkCommand).
-  for (auto &I : llvm::make_range(OpenMPTCRange.first, OpenMPTCRange.second)) {
-const ToolChain *TC = I.second;
-
-if (!TC->getTriple().isAMDGPU() || Args.hasArg(options::OPT_nogpulib))
-  continue;
-
-const ArgList &TCArgs = C.getArgsForToolChain(TC, "", Action::OFK_OpenMP);
-StringRef Arch = TCArgs.getLastArgValue(options::OPT_march_EQ);
-const toolchains::ROCMToolChain RocmTC(TC->getDriver(), TC->getTriple(),
-   TCArgs);
-
-SmallVector BCLibs =
-RocmTC.getCommonDeviceLibNames(TCArgs, Arch.str());
-
-for (StringRef LibName : BCLibs)
-  CmdArgs.push_back(Args.MakeArgString(
-  "--bitcode-library=" +
-  Action::GetOffloadKindName(Action::OFK_OpenMP) + "-" +
-  TC->getTripleString() + "-" + Arch + "=" + LibName));
-  }
-
   if (D.isUsingLTO(/* IsOffload */ true)) {
 // Pass in the optimization level to use for LTO.
 if (const Arg *A = Args.getLastArg(options::OPT_O_Group)) {
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
===
--- clang/lib/Driver/ToolCh

[PATCH] D133586: [clang] do not hash undefined qualifiers for FunctionNoProtoType

[Richard Howell via Phabricator via cfe-commits]

rmaz planned changes to this revision.
rmaz added a comment.

I think it is probably safer to zero out the FastTypeQuals instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133586

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

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[David Rector via Phabricator via cfe-commits]

davrec added inline comments.



Comment at: clang/include/clang/AST/Type.h:5530-5537
 /// Represents a type that was referred to using an elaborated type
 /// keyword, e.g., struct S, or via a qualified name, e.g., N::M::type,
 /// or both.
 ///
 /// This type is used to keep track of a type name as written in the
 /// source code, including tag keywords and any nested-name-specifiers.
 /// The type itself is always "sugar", used to express what was written

This documentation needs to be updated given the changes in this patch.  
Suggest you remove the first paragraph and flesh out the second paragraph, e.g.

```
/// A sugar type used to keep track of a type name as written in the
/// source code, including any tag keywords (e.g. struct S) and/or any 
/// nested-name-specifiers (e.g. N::M::type).  Note it will even be created
/// for types written *without* tag words or nested-name-specifiers, to
/// properly represent their absence in the written code.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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