[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

[Chuanqi Xu via Phabricator via cfe-commits]

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

Use ODRHash to simplify code.


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

https://reviews.llvm.org/D118034

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Modules/Inputs/redundant-template-default-arg/foo.cppm
  clang/test/Modules/Inputs/redundant-template-default-arg/foo.h
  clang/test/Modules/Inputs/redundant-template-default-arg2/foo.cppm
  clang/test/Modules/Inputs/redundant-template-default-arg3/foo.cppm
  clang/test/Modules/Inputs/redundant-template-default-arg3/foo.h
  clang/test/Modules/Inputs/redundant-template-default-arg3/foo_bad.h
  clang/test/Modules/redundant-template-default-arg.cpp
  clang/test/Modules/redundant-template-default-arg2.cpp
  clang/test/Modules/redundant-template-default-arg3.cpp

Index: clang/test/Modules/redundant-template-default-arg3.cpp
===
--- /dev/null
+++ clang/test/Modules/redundant-template-default-arg3.cpp
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang_cc1  -std=c++20 %S/Inputs/redundant-template-default-arg3/foo.cppm -I%S/Inputs/redundant-template-default-arg3/. -emit-module-interface -o %t/foo.pcm
+// RUN: %clang_cc1  -fprebuilt-module-path=%t -std=c++20 %s -I%S/Inputs/redundant-template-default-arg3/. -fsyntax-only -verify
+
+import foo;
+#include "foo_bad.h"
+
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:1 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:1 {{previous default template argument defined in module foo.}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:4 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:4 {{previous default template argument defined in module foo.}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:10 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:10 {{previous default template argument defined in module foo.}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:17 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:13 {{previous default template argument defined in module foo.}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:23 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:16 {{previous default template argument defined in module foo.}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:27 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:20 {{previous default template argument defined in module foo.}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:31 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:24 {{previous default template argument defined in module foo.}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:34 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:27 {{previous default template argument defined in module foo.}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:40 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:33 {{previous default template argument defined in module foo.}}
Index: clang/test/Modules/redundant-template-default-arg2.cpp
===
--- /dev/null
+++ clang/test/Modules/redundant-template-default-arg2.cpp
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang_cc1  -std=c++20 %S/Inputs/redundant-template-default-arg2/foo.cppm -I%S/Inputs/redundant-template-default-arg2/. -emit-module-interface -o %t/foo.pcm
+// RUN: %clang_cc1  -fprebuilt-module-path=%t -std=c++20 %s -fsyntax-only -verify
+import foo;
+template 
+T v; // expected-error {{declaration of 'v' in the global module follows declaration in module foo}}
+ // expected-note@Inputs/redundant-template-default-arg2/foo.cppm:3 {{previous declaration is here}}
+
+template 
+int v2; // expected-error {{declaration of 'v2' in the global module follows declaration in module foo}}
+// expected-note@Inputs/redundant-template-default-arg2/foo.cppm:6 {{previous declaration is here}}
+
+template 
+class my_array {}; // expected-er

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682
+Expr::EvalResult EVRX, EVRY;
+if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) ||
+!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C))
+  return false;
+
+APValue VX = EVRX.Val, VY = EVRY.Val;
+if (VX.getKind() != VY.getKind())

urnathan wrote:
> I'm kind of surprised how complex this check is.  Isn't there an AST 
> comparator available somewhere?
I found ODRHash. I think it is much better now.


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

https://reviews.llvm.org/D118034

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

[PATCH] D114714: [C++20][Modules][2/8] Add enumerations for partition modules and stream them.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114714

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

[PATCH] D119949: [Clang-tidy] Check the existence of ElaboratedType's qualifiers

[Jun Zhang via Phabricator via cfe-commits]

junaire updated this revision to Diff 409510.
junaire added a comment.

- Only suppress canonical types when it's a TypedefNameType.
- Add more tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119949

Files:
  
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
@@ -198,6 +198,28 @@
   h<4>();
 }
 
+struct SP {
+  static int I;
+} P;
+
+void usep() {
+  P.I;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+  // CHECK-FIXES: {{^}}  SP::I;{{$}}
+}
+
+namespace NSP {
+struct SP {
+  static int I;
+} P;
+} // namespace NSP
+
+void usensp() {
+  NSP::P.I;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+  // CHECK-FIXES: {{^}}  NSP::SP::I;{{$}}
+}
+
 // Overloaded member access operator
 struct Q {
   static int K;
@@ -237,9 +259,9 @@
 
 namespace Outer {
   inline namespace Inline {
-struct S {
-  static int I;
-};
+  struct S {
+static int I;
+  };
   }
 }
 
Index: 
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
+++ 
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
@@ -19,14 +19,15 @@
 
 static unsigned getNameSpecifierNestingLevel(const QualType &QType) {
   if (const ElaboratedType *ElType = QType->getAs()) {
-const NestedNameSpecifier *NestedSpecifiers = ElType->getQualifier();
-unsigned NameSpecifierNestingLevel = 1;
-do {
-  NameSpecifierNestingLevel++;
-  NestedSpecifiers = NestedSpecifiers->getPrefix();
-} while (NestedSpecifiers);
-
-return NameSpecifierNestingLevel;
+if (const NestedNameSpecifier *NestedSpecifiers = ElType->getQualifier()) {
+  unsigned NameSpecifierNestingLevel = 1;
+  do {
+NameSpecifierNestingLevel++;
+NestedSpecifiers = NestedSpecifiers->getPrefix();
+  } while (NestedSpecifiers);
+
+  return NameSpecifierNestingLevel;
+}
   }
   return 0;
 }
@@ -68,6 +69,11 @@
   PrintingPolicy PrintingPolicyWithSupressedTag(AstContext->getLangOpts());
   PrintingPolicyWithSupressedTag.SuppressTagKeyword = true;
   PrintingPolicyWithSupressedTag.SuppressUnwrittenScope = true;
+
+  bool ShouldPrintCanonicalTy =
+  BaseExpr->getType()->isTypedefNameType() ? false : true;
+  PrintingPolicyWithSupressedTag.PrintCanonicalTypes = ShouldPrintCanonicalTy;
+
   std::string BaseTypeName =
   BaseType.getAsString(PrintingPolicyWithSupressedTag);
 


Index: clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
@@ -198,6 +198,28 @@
   h<4>();
 }
 
+struct SP {
+  static int I;
+} P;
+
+void usep() {
+  P.I;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+  // CHECK-FIXES: {{^}}  SP::I;{{$}}
+}
+
+namespace NSP {
+struct SP {
+  static int I;
+} P;
+} // namespace NSP
+
+void usensp() {
+  NSP::P.I;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+  // CHECK-FIXES: {{^}}  NSP::SP::I;{{$}}
+}
+
 // Overloaded member access operator
 struct Q {
   static int K;
@@ -237,9 +259,9 @@
 
 namespace Outer {
   inline namespace Inline {
-struct S {
-  static int I;
-};
+  struct S {
+static int I;
+  };
   }
 }
 
Index: clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
@@ -19,14 +19,15 @@
 
 static unsigned getNameSpecifierNestingLevel(const QualType &QType) {
   if (const ElaboratedType *ElType = QType->getAs()) {
-const NestedNameSpecifier *NestedSpecifiers = ElType->getQualifier();
-unsigned NameSpecifierNestingLevel = 1;
-do {
-  NameSpecifierNestingLevel++;
-  NestedSpecifiers = NestedSpecifiers->getPrefix();
-} while (NestedSpecifiers);
-
-return NameSpecifierNestingLevel;
+if (const NestedNameSpecifier *NestedSpecifiers = ElType->getQualifier()) {
+  unsigned NameSpecifierNestingLevel

[PATCH] D119949: [Clang-tidy] Check the existence of ElaboratedType's qualifiers

[Jun Zhang via Phabricator via cfe-commits]

junaire updated this revision to Diff 409511.
junaire added a comment.

Simplify code a little bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119949

Files:
  
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
@@ -198,6 +198,28 @@
   h<4>();
 }
 
+struct SP {
+  static int I;
+} P;
+
+void usep() {
+  P.I;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+  // CHECK-FIXES: {{^}}  SP::I;{{$}}
+}
+
+namespace NSP {
+struct SP {
+  static int I;
+} P;
+} // namespace NSP
+
+void usensp() {
+  NSP::P.I;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+  // CHECK-FIXES: {{^}}  NSP::SP::I;{{$}}
+}
+
 // Overloaded member access operator
 struct Q {
   static int K;
@@ -237,9 +259,9 @@
 
 namespace Outer {
   inline namespace Inline {
-struct S {
-  static int I;
-};
+  struct S {
+static int I;
+  };
   }
 }
 
Index: 
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
+++ 
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
@@ -19,14 +19,15 @@
 
 static unsigned getNameSpecifierNestingLevel(const QualType &QType) {
   if (const ElaboratedType *ElType = QType->getAs()) {
-const NestedNameSpecifier *NestedSpecifiers = ElType->getQualifier();
-unsigned NameSpecifierNestingLevel = 1;
-do {
-  NameSpecifierNestingLevel++;
-  NestedSpecifiers = NestedSpecifiers->getPrefix();
-} while (NestedSpecifiers);
-
-return NameSpecifierNestingLevel;
+if (const NestedNameSpecifier *NestedSpecifiers = ElType->getQualifier()) {
+  unsigned NameSpecifierNestingLevel = 1;
+  do {
+NameSpecifierNestingLevel++;
+NestedSpecifiers = NestedSpecifiers->getPrefix();
+  } while (NestedSpecifiers);
+
+  return NameSpecifierNestingLevel;
+}
   }
   return 0;
 }
@@ -68,6 +69,10 @@
   PrintingPolicy PrintingPolicyWithSupressedTag(AstContext->getLangOpts());
   PrintingPolicyWithSupressedTag.SuppressTagKeyword = true;
   PrintingPolicyWithSupressedTag.SuppressUnwrittenScope = true;
+
+  PrintingPolicyWithSupressedTag.PrintCanonicalTypes =
+  BaseExpr->getType()->isTypedefNameType() ? false : true;
+
   std::string BaseTypeName =
   BaseType.getAsString(PrintingPolicyWithSupressedTag);
 


Index: clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
@@ -198,6 +198,28 @@
   h<4>();
 }
 
+struct SP {
+  static int I;
+} P;
+
+void usep() {
+  P.I;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+  // CHECK-FIXES: {{^}}  SP::I;{{$}}
+}
+
+namespace NSP {
+struct SP {
+  static int I;
+} P;
+} // namespace NSP
+
+void usensp() {
+  NSP::P.I;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+  // CHECK-FIXES: {{^}}  NSP::SP::I;{{$}}
+}
+
 // Overloaded member access operator
 struct Q {
   static int K;
@@ -237,9 +259,9 @@
 
 namespace Outer {
   inline namespace Inline {
-struct S {
-  static int I;
-};
+  struct S {
+static int I;
+  };
   }
 }
 
Index: clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
@@ -19,14 +19,15 @@
 
 static unsigned getNameSpecifierNestingLevel(const QualType &QType) {
   if (const ElaboratedType *ElType = QType->getAs()) {
-const NestedNameSpecifier *NestedSpecifiers = ElType->getQualifier();
-unsigned NameSpecifierNestingLevel = 1;
-do {
-  NameSpecifierNestingLevel++;
-  NestedSpecifiers = NestedSpecifiers->getPrefix();
-} while (NestedSpecifiers);
-
-return NameSpecifierNestingLevel;
+if (const NestedNameSpecifier *NestedSpecifiers = ElType->getQualifier()) {
+  unsigned NameSpecifierNestingLevel = 1;
+  do {
+NameSpecifierNestingLevel++;
+NestedSpecifiers = NestedSpecifiers->getPre

[clang] 0ae2464 - [clang-format] Fix wrong assertion with non-negative shift when aligning tokens.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-02-17T09:49:00+01:00
New Revision: 0ae2464fcd4d2c2f285b83d16ff6e2426dd722d2

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

LOG: [clang-format] Fix wrong assertion with non-negative shift when aligning 
tokens.

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

Added: 


Modified: 
clang/lib/Format/WhitespaceManager.cpp
clang/unittests/Format/FormatTestSelective.cpp

Removed: 




diff  --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index 758dc5860888e..55e0b7f8e8d9e 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -406,7 +406,7 @@ AlignTokenSequence(const FormatStyle &Style, unsigned 
Start, unsigned End,
   Changes[i].Spaces += Shift;
 
 // We should not remove required spaces unless we break the line before.
-assert(Changes[i].NewlinesBefore > 0 ||
+assert(Shift >= 0 || Changes[i].NewlinesBefore > 0 ||
Changes[i].Spaces >=
static_cast(Changes[i].Tok->SpacesRequiredBefore) ||
Changes[i].Tok->is(tok::eof));

diff  --git a/clang/unittests/Format/FormatTestSelective.cpp 
b/clang/unittests/Format/FormatTestSelective.cpp
index c88d1b8bd8ba2..2725e4cf776f6 100644
--- a/clang/unittests/Format/FormatTestSelective.cpp
+++ b/clang/unittests/Format/FormatTestSelective.cpp
@@ -603,6 +603,14 @@ TEST_F(FormatTestSelective, 
KeepsIndentAfterCommentSectionImport) {
   EXPECT_EQ(Code, format(Code, 47, 1));
 }
 
+TEST_F(FormatTestSelective, DontAssert) {
+  // https://llvm.org/PR53880
+  std::string Code = "void f() {\n"
+ "  return a == 8 ? 32 : 16;\n"
+ "}\n";
+  EXPECT_EQ(Code, format(Code, 40, 0));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang



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

[PATCH] D118893: [C++20][Modules][1/8] Track valid import state.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 409521.
iains added a comment.

address review comments, rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118893

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/Modules/cxx20-import-diagnostics-a.cpp

Index: clang/test/Modules/cxx20-import-diagnostics-a.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -0,0 +1,140 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=0 -x c++ %s \
+// RUN:  -o %t/B.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN:  -o %t/C.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -fmodule-file=%t/C.pcm -o %t/AOK1.pcm
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=3 -x c++ %s \
+// RUN:  -fmodule-file=%t/AOK1.pcm -o %t/tu_3.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=4 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -fmodule-file=%t/C.pcm -o %t/BC.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=5 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -fmodule-file=%t/C.pcm -o %t/tu_5.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=6 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -o %t/D.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=7 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -o %t/D.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=8 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -o %t/tu_8.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=9 -x c++ %s \
+// RUN:  -o %t/B.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -D TU=10 -x c++ %s \
+// RUN:  -fmodule-file=%t/C.pcm  -o %t/impl.o
+
+// Test diagnostics for incorrect module import sequences.
+
+#if TU == 0
+
+export module B;
+
+int foo ();
+
+// expected-no-diagnostics
+
+#elif TU == 1
+
+export module C;
+
+int bar ();
+
+// expected-no-diagnostics
+
+#elif TU == 2
+
+export module AOK1;
+
+import B;
+export import C;
+
+export int theAnswer ();
+
+// expected-no-diagnostics
+
+#elif TU == 3
+
+module;
+
+module AOK1;
+
+export import C; // expected-error {{export declaration can only be used within a module interface unit}}
+
+int theAnswer () { return 42; }
+
+#elif TU == 4
+
+export module BC;
+
+export import B;
+
+int foo () { return 10; }
+
+import C; // expected-error {{imports must immediately follow the module declaration}}
+
+#elif TU == 5
+
+module B; // implicitly imports B.
+
+int foo () { return 10; }
+
+import C; // expected-error {{imports must immediately follow the module declaration}}
+
+#elif TU == 6
+
+module;
+// We can only have preprocessor commands here, which could include an include
+// translated header unit.  However those are identified specifically by the
+// preprocessor; non-preprocessed user code should not contain an import here.
+import B; // expected-error {{module imports cannot be in the global module fragment}}
+
+export module D;
+
+int delta ();
+
+#elif TU == 7
+
+export module D;
+
+int delta ();
+
+module :private;
+
+import B; // expected-error {{module imports cannot be in the private module fragment}}
+
+#elif TU == 8
+
+module B;
+
+import B; // expected-error {{import of module 'B' appears within same top-level module 'B'}}
+
+#elif TU == 9
+
+export module B;
+
+import B; // expected-error {{import of module 'B' appears within same top-level module 'B'}}
+
+#elif TU == 10
+
+int x;
+
+import C;
+
+int baz() { return 6174; }
+
+// expected-no-diagnostics
+
+#else
+#error "no MODE set"
+#endif
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -80,12 +80,20 @@
   return nullptr;
 }
 
-Sema::DeclGroupPtrTy
-Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
-  ModuleDeclKind MDK, ModuleIdPath Path, bool IsFirstDecl) {
+Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation StartLoc,
+   SourceLocation ModuleLoc,
+   ModuleDeclKind MDK,
+   ModuleIdPath Path,
+   ModuleImportState &ImportState) {
   assert((getLangOpts().ModulesTS || getLangOpts().CPlusPlusModules) &&
  "should only have module decl in Modules TS or C++20");
 
+  bool IsFirstDecl = ImportState == ModuleImportState::FirstDecl;
+  bool SeenGMF = ImportState 

[PATCH] D118893: [C++20][Modules][1/8] Track valid import state.

[Iain Sandoe via Phabricator via cfe-commits]

iains marked 5 inline comments as done.
iains added inline comments.



Comment at: clang/lib/Parse/ParseObjc.cpp:82
 if (getLangOpts().Modules || getLangOpts().DebuggerSupport) {
-  SingleDecl = ParseModuleImport(AtLoc);
+  Sema::ModuleImportState IS = Sema::ModuleImportState::NotACXX20Module;
+  SingleDecl = ParseModuleImport(AtLoc, IS);

ChuanqiXu wrote:
> We could remove the IS variable
ParseModuleImport  takes a reference so that it can update the the 
ModuleImportState if an invalid state is detected in parsing.  So, although the 
state variable is a dummy here, we cannot remove the IS var.



Comment at: clang/lib/Parse/Parser.cpp:913
+}
+SingleDecl = ParseModuleImport(SourceLocation(), IS);
+  } break;

ChuanqiXu wrote:
> I think it is better to:
> ```
> SingleDecl = ParseModuleImport(SourceLocation(), 
> ModuleImportState::NotACXX20Module);
> ```
see comment above, ParseModuleImport takes a reference.



Comment at: clang/lib/Parse/Parser.cpp:2466
+  case Sema::ModuleImportState::NotACXX20Module:
+// These cases will be an error when partitions are implemented.
+SeenError = false;

ChuanqiXu wrote:
> Generally, we would add a `TODO` or `FIXME` for such comments.
> (This is not need to be addressed if the following patch would be landed 
> quickly)
I hope that the main changes will be landed soon, but adding the TODO anyway.



Comment at: clang/lib/Sema/SemaModule.cpp:144
 GlobalModuleFragment = ModuleScopes.back().Module;
 
   // In C++20, the module-declaration must be the first declaration if there

ChuanqiXu wrote:
> I feel better to add an assertion here:
> ```
> assert(SeenGMF == GlobalModuleFragment && "msg);
> ```
we also have to check that we are in C++20 modules mode, since implicit global 
module fragments are allowed elsewhere - the test below already guards on C++20 
modules.

I've made this change, although the assert seems quite heavyweight.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118893

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

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

I think we should add 2 more tests at least:
Import a partition a module purview but we failed to find one. And test 
reflects the successful case.




Comment at: clang/include/clang/Sema/Sema.h:2989
   /// \param ImportLoc The location of the 'import' keyword.
-  /// \param Path The module access path.
+  /// \param NamePath The module toplevel name as an access path.
+  /// \param Partition The module partition name as an access path.

urnathan wrote:
> Is `NamePath` really a better name?  You've not consistently changed all 
> `Path`'s to this, and it doesn;t strike me as particularly mnemonic.
I use `Path` in my implementation.



Comment at: clang/lib/Sema/SemaDecl.cpp:1611
 
+  // If we have a decl in a module partition it is part of the containing
+  // module (which is the only thing that can be importing it).

We lack a comma here.



Comment at: clang/lib/Sema/SemaModule.cpp:224-225
+if (IsPartition) {
+  // Create an interface, but note that it is an implementation
+  // unit.
   Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName,

I prefer to add more words here to tell why we create an interface unit but the 
above condition is `ModuleDeclKind::Implementation`. 



Comment at: clang/lib/Sema/SemaModule.cpp:260
+  ModuleScopes.back().ModuleInterface =
+  (MDK != ModuleDeclKind::Implementation || IsPartition);
+  ModuleScopes.back().IsPartition = IsPartition;

How about: `Mod->Kind != Module::ModulePartitionImplementation`.



Comment at: clang/lib/Sema/SemaModule.cpp:346-347
SourceLocation ImportLoc,
-   ModuleIdPath Path) {
-  // Flatten the module path for a C++20 or Modules TS module name.
+   ModuleIdPath NamePath,
+   ModuleIdPath Partition) {
+

I think we could change the signature of the lat 2 fields to 
```
ModuleIdPath Path,
bool IsPartition)
```
I feel this is more simpler.



Comment at: clang/lib/Sema/SemaModule.cpp:350
+  bool IsPartition = !Partition.empty();
+  bool Cxx20Mode = getLangOpts().CPlusPlusModules || getLangOpts().ModulesTS;
+  assert((!IsPartition || Cxx20Mode) && "partition seen in non-C++20 code?");

I prefer to remove the support for getLangOpts().ModulesTS on the fly.



Comment at: clang/lib/Sema/SemaModule.cpp:359
+
   std::string ModuleName;
+  if (IsPartition) {

I prefer to move the variable to the following block.



Comment at: clang/lib/Sema/SemaModule.cpp:361
+  if (IsPartition) {
+// We already checked that we are in a module purview in the parser.
+assert(!ModuleScopes.empty() && "in a module purview, but no module?");

I prefer to add an assertion:
```
assert (ModuleScopes.back().Module.isModulePurview());
```



Comment at: clang/lib/Sema/SemaModule.cpp:367-368
+  // owning named module.
+  size_t P = NamedMod->Name.find_first_of(":");
+  ModuleName = NamedMod->Name.substr(0, P + 1);
+} else {

I would like to wrap this one as a method of class `Module`:
```
StringRef getPrimaryModuleName(Module *M) {
if (not partition)
   return M->Name;
return M->Name.split(":").first;
}
```



Comment at: clang/lib/Sema/SemaModule.cpp:373
+  ModuleName = NamedMod->Name;
+  ModuleName += ":";
 }

We could move this below the if statement.



Comment at: clang/lib/Sema/SemaModule.cpp:433-451
+  if (NamePath.empty()) {
+// If this was a header import, pad out with dummy locations.
+// FIXME: Pass in and use the location of the header-name token in this
+// case.
+for (Module *ModCheck = Mod; ModCheck; ModCheck = ModCheck->Parent)
   IdentifierLocs.push_back(SourceLocation());
+  } else if (getLangOpts().CPlusPlusModules && !Mod->Parent) {

I don't find the reason to refactor here. It looks NFC and I think the original 
form is simpler.



Comment at: clang/lib/Sema/SemaModule.cpp:477-480
+  } else if (getLangOpts().isCompilingModule()) {
+Module *ThisModule = PP.getHeaderSearchInfo().lookupModule(
+getLangOpts().CurrentModule, ExportLoc, false, false);
+assert(ThisModule && "was expecting a module if building one");

What does it assert? I don't get the point.



Comment at: clang/test/Modules/cxx20-import-diagnostics-a.cpp:57
 
+module;
+

What if we don't add this? I think the original is good. So we should add a new 
test if we feel needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118586

__

[PATCH] D118893: [C++20][Modules][1/8] Track valid import state.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added a comment.

LGTM.




Comment at: clang/lib/Sema/SemaModule.cpp:144
 GlobalModuleFragment = ModuleScopes.back().Module;
 
   // In C++20, the module-declaration must be the first declaration if there

iains wrote:
> ChuanqiXu wrote:
> > I feel better to add an assertion here:
> > ```
> > assert(SeenGMF == GlobalModuleFragment && "msg);
> > ```
> we also have to check that we are in C++20 modules mode, since implicit 
> global module fragments are allowed elsewhere - the test below already guards 
> on C++20 modules.
> 
> I've made this change, although the assert seems quite heavyweight.
nit: Is the 
```
(SeenGMF == (GlobalModuleFragment != nullptr))
```
not equal to:
```
SeenGMF == GlobalModuleFragment
``` 
? Or we could add a cast here:
```
SeenGMF == (bool) GlobalModuleFragment
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118893

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

[clang] dd4dde8 - [clang][dataflow] Add transfer functions for logical and, or, not.

[Stanislav Gatev via cfe-commits]

Author: Stanislav Gatev
Date: 2022-02-17T09:09:59Z
New Revision: dd4dde8d39a9c36ea692635bdfc0c90cc8d755fd

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

LOG: [clang][dataflow] Add transfer functions for logical and, or, not.

This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.

Reviewed-by: xazax.hun

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/Transfer.h
clang/include/clang/Analysis/FlowSensitive/Value.h
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Removed: 




diff  --git 
a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index 5c1b41d538921..52f738d59b812 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -34,8 +34,8 @@ namespace dataflow {
 class DataflowAnalysisContext {
 public:
   DataflowAnalysisContext()
-  : TrueVal(&takeOwnership(std::make_unique())),
-FalseVal(&takeOwnership(std::make_unique())) {}
+  : TrueVal(takeOwnership(std::make_unique())),
+FalseVal(takeOwnership(std::make_unique())) {}
 
   /// Takes ownership of `Loc` and returns a reference to it.
   ///
@@ -115,8 +115,8 @@ class DataflowAnalysisContext {
 
   /// Returns a symbolic boolean value that models a boolean literal equal to
   /// `Value`.
-  BoolValue &getBoolLiteralValue(bool Value) const {
-return Value ? *TrueVal : *FalseVal;
+  AtomicBoolValue &getBoolLiteralValue(bool Value) const {
+return Value ? TrueVal : FalseVal;
   }
 
 private:
@@ -135,8 +135,8 @@ class DataflowAnalysisContext {
   StorageLocation *ThisPointeeLoc = nullptr;
 
   // FIXME: Add support for boolean expressions.
-  BoolValue *TrueVal;
-  BoolValue *FalseVal;
+  AtomicBoolValue &TrueVal;
+  AtomicBoolValue &FalseVal;
 };
 
 } // namespace dataflow

diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index cebfb66ef242f..af613c95bb8dc 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -226,7 +226,7 @@ class Environment {
 
   /// Returns a symbolic boolean value that models a boolean literal equal to
   /// `Value`
-  BoolValue &getBoolLiteralValue(bool Value) const {
+  AtomicBoolValue &getBoolLiteralValue(bool Value) const {
 return DACtx->getBoolLiteralValue(Value);
   }
 

diff  --git a/clang/include/clang/Analysis/FlowSensitive/Transfer.h 
b/clang/include/clang/Analysis/FlowSensitive/Transfer.h
index a12674a173be4..a6b663b997fd6 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Transfer.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Transfer.h
@@ -20,12 +20,23 @@
 namespace clang {
 namespace dataflow {
 
+/// Maps statements to the environments of basic blocks that contain them.
+class StmtToEnvMap {
+public:
+  virtual ~StmtToEnvMap() = default;
+
+  /// Returns the environment of the basic block that contains `S` or nullptr 
if
+  /// there isn't one.
+  /// FIXME: Ensure that the result can't be null and return a const reference.
+  virtual const Environment *getEnvironment(const Stmt &S) const = 0;
+};
+
 /// Evaluates `S` and updates `Env` accordingly.
 ///
 /// Requirements:
 ///
 ///  The type of `S` must not be `ParenExpr`.
-void transfer(const Stmt &S, Environment &Env);
+void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env);
 
 } // namespace dataflow
 } // namespace clang

diff  --git a/clang/include/clang/Analysis/FlowSensitive/Value.h 
b/clang/include/clang/Analysis/FlowSensitive/Value.h
index da04f926c597b..7c02cc6c3505b 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Value.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -28,7 +28,19 @@ namespace dataflow {
 /// Base class for all values computed by abstract interpretation.
 class Value {
 public:
-  enum class Kind { Bool, Integer, Reference, Pointer, Struct };
+  enum class Kind {
+Integer,
+Reference,
+Pointer,
+Struct,
+
+// Synthetic boolean values are either atomic values or composites that
+// represent conjunctions, disjunct

[PATCH] D119953: [clang][dataflow] Add transfer functions for logical and, or, not.

[Stanislav Gatev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdd4dde8d39a9: [clang][dataflow] Add transfer functions for 
logical and, or, not. (authored by sgatev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119953

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/Transfer.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -387,7 +387,8 @@
 Code, "target",
 [this](ASTContext &Context, Environment &Env) {
   assert(HasValueTop == nullptr);
-  HasValueTop = &Env.takeOwnership(std::make_unique());
+  HasValueTop =
+  &Env.takeOwnership(std::make_unique());
   return OptionalIntAnalysis(Context, *HasValueTop);
 },
 [&Match](
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2006,6 +2006,42 @@
   // [[p]]
 }
   )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const auto *FooVal = dyn_cast_or_null(
+Env.getValue(*FooDecl, SkipPast::None));
+ASSERT_THAT(FooVal, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarVal = dyn_cast_or_null(
+Env.getValue(*BarDecl, SkipPast::None));
+ASSERT_THAT(BarVal, NotNull());
+
+EXPECT_EQ(FooVal, &Env.getBoolLiteralValue(true));
+EXPECT_EQ(BarVal, &Env.getBoolLiteralValue(false));
+  });
+}
+
+TEST_F(TransferTest, AssignFromBoolConjunction) {
+  std::string Code = R"(
+void target() {
+  bool Foo = true;
+  bool Bar = true;
+  bool Baz = (Foo) && (Bar);
+  // [[p]]
+}
+  )";
   runDataflow(
   Code, [](llvm::ArrayRef<
std::pair>>
@@ -2028,9 +2064,93 @@
 dyn_cast_or_null(Env.getValue(*BarDecl, SkipPast::None));
 ASSERT_THAT(BarVal, NotNull());
 
-EXPECT_EQ(FooVal, &Env.getBoolLiteralValue(true));
-EXPECT_EQ(BarVal, &Env.getBoolLiteralValue(false));
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *BazVal = dyn_cast_or_null(
+Env.getValue(*BazDecl, SkipPast::None));
+ASSERT_THAT(BazVal, NotNull());
+
+EXPECT_EQ(&BazVal->getLeftSubValue(), FooVal);
+EXPECT_EQ(&BazVal->getRightSubValue(), BarVal);
   });
 }
 
+TEST_F(TransferTest, AssignFromBoolDisjunction) {
+  std::string Code = R"(
+void target() {
+  bool Foo = true;
+  bool Bar = true;
+  bool Baz = (Foo) || (Bar);
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const auto *FooVal =
+dyn_cast_or_null(Env.getValue(*FooDecl, SkipPast::None));
+ASSERT_THAT(FooVal, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarVal =
+dyn_cast_or_null(Env.getValue(*BarDecl, SkipPast::None));
+ASSERT_THAT(BarVal, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+ 

[PATCH] D118893: [C++20][Modules][1/8] Track valid import state.

[Iain Sandoe via Phabricator via cfe-commits]

iains marked 4 inline comments as done.
iains added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:144
 GlobalModuleFragment = ModuleScopes.back().Module;
 
   // In C++20, the module-declaration must be the first declaration if there

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > I feel better to add an assertion here:
> > > ```
> > > assert(SeenGMF == GlobalModuleFragment && "msg);
> > > ```
> > we also have to check that we are in C++20 modules mode, since implicit 
> > global module fragments are allowed elsewhere - the test below already 
> > guards on C++20 modules.
> > 
> > I've made this change, although the assert seems quite heavyweight.
> nit: Is the 
> ```
> (SeenGMF == (GlobalModuleFragment != nullptr))
> ```
> not equal to:
> ```
> SeenGMF == GlobalModuleFragment
> ``` 
> ? Or we could add a cast here:
> ```
> SeenGMF == (bool) GlobalModuleFragment
> ```
> nit: Is the 

> ```
> SeenGMF == GlobalModuleFragment
> ``` 

clang complains of pointer / integer comparison.
I guess we could use the cast.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118893

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

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:177
+  if (IsPartition) {
+ModuleName += ":";
+ModuleName += stringFromPath(Partition);

I chose '-' in my implementation since here ModuleName refers to the name in 
filesystem, right? And I remember '-' is the choice of GCC. So let's keep 
consistency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118586

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

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/test/Modules/cxx20-partition-diagnostics-a.cpp:3-5
+// RUN: %clang_cc1 -std=c++20 -S -D TU=1 -x c++ %s -o /dev/null -verify
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=2 -x c++ %s -o /dev/null -verify

We could use `-fsyntax-only` instead of `-o /dev/null`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118586

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

[PATCH] D118587: [C++20][Modules][4/8] Handle generation of partition implementation CMIs.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/test/Modules/cxx20-10-1-ex1.cpp:7-8
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN:  -o %t/A_Internals.pcm
+

In my implementation, I replace ':' to '-' in pcm filename. I do so since I 
remember this is the behavior of GCC. I think '-' is better than '_' since '_' 
is possible to occur in name.

And I am surprised that the compiler wouldn't complain about that it couldn't 
find the corresponding pcm. Since I don't see corresponding code so far.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118587

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

[PATCH] D118588: [C++20][Modules]5/8] Diagnose wrong import/export for partition CMIs.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added a comment.

LGTM with the name about partition CMI addressed (It is raised in previous 
patch)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118588

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

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:367-368
+  // owning named module.
+  size_t P = NamedMod->Name.find_first_of(":");
+  ModuleName = NamedMod->Name.substr(0, P + 1);
+} else {

ChuanqiXu wrote:
> I would like to wrap this one as a method of class `Module`:
> ```
> StringRef getPrimaryModuleName(Module *M) {
> if (not partition)
>return M->Name;
> return M->Name.split(":").first;
> }
> ```
Oh, I found it made it in following patches. I think the series of patch is a 
little bit sparse.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118586

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

[PATCH] D118589: [C++20][Modules][6/8] Record direct module imports.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

hmmm, I don't find this is used in following patch. Oversight?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118589

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

[PATCH] D118598: [C++20][Modules][7/8] Find the primary interface name for a module.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

My implementation is

  StringRef getPrimaryModuleName() const {
  if (Check Kind)
return Name;
  return StringRef(Name).split('-').first;
}


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118598

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

[clang] a480841 - Add missing break statement in switch.

[Stanislav Gatev via cfe-commits]

Author: Stanislav Gatev
Date: 2022-02-17T09:37:02Z
New Revision: a48084156653e8f3791d13c19ce64df13c44a11e

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

LOG: Add missing break statement in switch.

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Transfer.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 72475e0c79d90..cd9b8b0e454e4 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -260,12 +260,13 @@ class TransferVisitor : public 
ConstStmtVisitor {
   auto *SubExprVal =
   dyn_cast_or_null(Env.getValue(*SubExpr, SkipPast::None));
   if (SubExprVal == nullptr)
-return;
+break;
 
   auto &ExprLoc = Env.createStorageLocation(*S);
   Env.setStorageLocation(*S, ExprLoc);
   Env.setValue(ExprLoc, Env.takeOwnership(
 std::make_unique(*SubExprVal)));
+  break;
 }
 default:
   break;



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

[PATCH] D118599: [C++20][Modules][8/8] Amend module visibility rules for partitions.

[Chuanqi Xu via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118599

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

[PATCH] D118589: [C++20][Modules][6/8] Record direct module imports.

[Chuanqi Xu via Phabricator via cfe-commits]

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

Sorry, I took an oversight.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118589

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

[PATCH] D118893: [C++20][Modules][1/8] Track valid import state.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 409546.
iains added a comment.

amended an assert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118893

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/Modules/cxx20-import-diagnostics-a.cpp

Index: clang/test/Modules/cxx20-import-diagnostics-a.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -0,0 +1,140 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=0 -x c++ %s \
+// RUN:  -o %t/B.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN:  -o %t/C.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -fmodule-file=%t/C.pcm -o %t/AOK1.pcm
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=3 -x c++ %s \
+// RUN:  -fmodule-file=%t/AOK1.pcm -o %t/tu_3.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=4 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -fmodule-file=%t/C.pcm -o %t/BC.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=5 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -fmodule-file=%t/C.pcm -o %t/tu_5.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=6 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -o %t/D.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=7 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -o %t/D.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=8 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -o %t/tu_8.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=9 -x c++ %s \
+// RUN:  -o %t/B.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -D TU=10 -x c++ %s \
+// RUN:  -fmodule-file=%t/C.pcm  -o %t/impl.o
+
+// Test diagnostics for incorrect module import sequences.
+
+#if TU == 0
+
+export module B;
+
+int foo ();
+
+// expected-no-diagnostics
+
+#elif TU == 1
+
+export module C;
+
+int bar ();
+
+// expected-no-diagnostics
+
+#elif TU == 2
+
+export module AOK1;
+
+import B;
+export import C;
+
+export int theAnswer ();
+
+// expected-no-diagnostics
+
+#elif TU == 3
+
+module;
+
+module AOK1;
+
+export import C; // expected-error {{export declaration can only be used within a module interface unit}}
+
+int theAnswer () { return 42; }
+
+#elif TU == 4
+
+export module BC;
+
+export import B;
+
+int foo () { return 10; }
+
+import C; // expected-error {{imports must immediately follow the module declaration}}
+
+#elif TU == 5
+
+module B; // implicitly imports B.
+
+int foo () { return 10; }
+
+import C; // expected-error {{imports must immediately follow the module declaration}}
+
+#elif TU == 6
+
+module;
+// We can only have preprocessor commands here, which could include an include
+// translated header unit.  However those are identified specifically by the
+// preprocessor; non-preprocessed user code should not contain an import here.
+import B; // expected-error {{module imports cannot be in the global module fragment}}
+
+export module D;
+
+int delta ();
+
+#elif TU == 7
+
+export module D;
+
+int delta ();
+
+module :private;
+
+import B; // expected-error {{module imports cannot be in the private module fragment}}
+
+#elif TU == 8
+
+module B;
+
+import B; // expected-error {{import of module 'B' appears within same top-level module 'B'}}
+
+#elif TU == 9
+
+export module B;
+
+import B; // expected-error {{import of module 'B' appears within same top-level module 'B'}}
+
+#elif TU == 10
+
+int x;
+
+import C;
+
+int baz() { return 6174; }
+
+// expected-no-diagnostics
+
+#else
+#error "no MODE set"
+#endif
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -80,12 +80,20 @@
   return nullptr;
 }
 
-Sema::DeclGroupPtrTy
-Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
-  ModuleDeclKind MDK, ModuleIdPath Path, bool IsFirstDecl) {
+Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation StartLoc,
+   SourceLocation ModuleLoc,
+   ModuleDeclKind MDK,
+   ModuleIdPath Path,
+   ModuleImportState &ImportState) {
   assert((getLangOpts().ModulesTS || getLangOpts().CPlusPlusModules) &&
  "should only have module decl in Modules TS or C++20");
 
+  bool IsFirstDecl = ImportState == ModuleImportState::FirstDecl;
+  bool SeenGMF = ImportState == ModuleImpo

[PATCH] D114714: [C++20][Modules][2/8] Add enumerations for partition modules and stream them.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 409548.
iains marked an inline comment as done.
iains added a comment.

rebased onto changes in parent patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114714

Files:
  clang/include/clang/Basic/Module.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/lib/Serialization/ASTWriter.cpp


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2674,7 +2674,7 @@
   Abbrev->Add(BitCodeAbbrevOp(SUBMODULE_DEFINITION));
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // ID
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Parent
-  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // Kind
+  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // Kind
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsFramework
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsExplicit
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsSystem
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -261,6 +261,8 @@
: ModuleScopes.back().Module->Kind) {
   case Module::ModuleMapModule:
   case Module::GlobalModuleFragment:
+  case Module::ModulePartitionImplementation:
+  case Module::ModulePartitionInterface:
 Diag(PrivateLoc, diag::err_private_module_fragment_not_module);
 return nullptr;
 
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -1550,6 +1550,8 @@
 return nullptr;
 
   case Module::ModuleInterfaceUnit:
+  case Module::ModulePartitionInterface:
+  case Module::ModulePartitionImplementation:
 return M;
 
   case Module::GlobalModuleFragment: {
Index: clang/include/clang/Basic/Module.h
===
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -106,9 +106,15 @@
 /// of header files.
 ModuleMapModule,
 
-/// This is a C++ Modules TS module interface unit.
+/// This is a C++20 module interface unit.
 ModuleInterfaceUnit,
 
+/// This is a C++ 20 module partition interface.
+ModulePartitionInterface,
+
+/// This is a C++ 20 module partition implementation.
+ModulePartitionImplementation,
+
 /// This is a fragment of the global module within some C++ module.
 GlobalModuleFragment,
 
@@ -150,7 +156,9 @@
 
   /// Does this Module scope describe part of the purview of a named C++ 
module?
   bool isModulePurview() const {
-return Kind == ModuleInterfaceUnit || Kind == PrivateModuleFragment;
+return Kind == ModuleInterfaceUnit || Kind == ModulePartitionInterface ||
+   Kind == ModulePartitionImplementation ||
+   Kind == PrivateModuleFragment;
   }
 
   /// Does this Module scope describe a fragment of the global module within
@@ -506,6 +514,9 @@
 Parent->SubModules.push_back(this);
   }
 
+  /// Is this a module partition.
+  bool isModulePartition() const { return Name.find(':') != std::string::npos; 
}
+
   /// Retrieve the full name of this module, including the path from
   /// its top-level module.
   /// \param AllowStringLiterals If \c true, components that might not be


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2674,7 +2674,7 @@
   Abbrev->Add(BitCodeAbbrevOp(SUBMODULE_DEFINITION));
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // ID
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Parent
-  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // Kind
+  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // Kind
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsFramework
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsExplicit
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsSystem
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -261,6 +261,8 @@
: ModuleScopes.back().Module->Kind) {
   case Module::ModuleMapModule:
   case Module::GlobalModuleFragment:
+  case Module::ModulePartitionImplementation:
+  case Module::ModulePartitionInterface:
 Diag(PrivateLoc, diag::err_private_module_fragment_not_module);
 return nullptr;
 
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 409552.
iains added a comment.

address review comments, rebase onto parent patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118586

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticIDs.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CXX/module/module.unit/p3.cpp
  clang/test/CXX/module/module.unit/p8.cpp
  clang/test/Modules/cxx20-import-diagnostics-a.cpp
  clang/test/Modules/cxx20-partition-diagnostics-a.cpp

Index: clang/test/Modules/cxx20-partition-diagnostics-a.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-partition-diagnostics-a.cpp
@@ -0,0 +1,18 @@
+// Module Partition diagnostics
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=1 -x c++ %s -o /dev/null -verify
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=2 -x c++ %s -o /dev/null -verify
+
+#if TU == 1
+
+import :B; // expected-error {{module partition imports must be within a module purview}}
+
+#elif TU == 2
+module; // expected-error {{missing 'module' declaration at end of global module fragment introduced here}}
+
+import :Part; // expected-error {{module partition imports cannot be in the global module fragment}}
+
+#else
+#error "no TU set"
+#endif
Index: clang/test/Modules/cxx20-import-diagnostics-a.cpp
===
--- clang/test/Modules/cxx20-import-diagnostics-a.cpp
+++ clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -54,6 +54,8 @@
 
 #elif TU == 2
 
+module;
+
 export module AOK1;
 
 import B;
Index: clang/test/CXX/module/module.unit/p8.cpp
===
--- clang/test/CXX/module/module.unit/p8.cpp
+++ clang/test/CXX/module/module.unit/p8.cpp
@@ -12,7 +12,7 @@
 
 #elif MODE == 1
 // expected-no-diagnostics
-module foo;
+module foo; // Implementation, implicitly imports foo.
 #define IMPORTED
 
 #elif MODE == 2
@@ -21,15 +21,15 @@
 #define IMPORTED
 
 #elif MODE == 3
-export module bar;
+export module bar; // A different module
 
 #elif MODE == 4
-module foo:bar; // expected-error {{not yet supported}}
-#define IMPORTED // FIXME
+module foo:bar; // Partition implementation
+//#define IMPORTED (we don't import foo here)
 
 #elif MODE == 5
-export module foo:bar; // expected-error {{not yet supported}} expected-error {{redefinition}} expected-note@* {{loaded from}}
-#define IMPORTED // FIXME
+export module foo:bar; // Partition interface
+//#define IMPORTED  (we don't import foo here)
 
 #endif
 
Index: clang/test/CXX/module/module.unit/p3.cpp
===
--- clang/test/CXX/module/module.unit/p3.cpp
+++ clang/test/CXX/module/module.unit/p3.cpp
@@ -1,4 +1,4 @@
 // RUN: %clang_cc1 -std=c++2a -verify %s
 
-export module foo:bar; // expected-error {{sorry, module partitions are not yet supported}}
-import :baz; // expected-error {{sorry, module partitions are not yet supported}}
+export module foo:bar;
+import :baz; // expected-error {{module 'foo:baz' not found}}
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -54,6 +54,23 @@
   }
 }
 
+// We represent the primary and partition names as 'Paths' which are sections
+// of the hierarchical access path for a clang module.  However for C++20
+// the periods in a name are just another character, and we will need to
+// flatten them into a string.
+static std::string stringFromPath(ModuleIdPath Path) {
+  std::string Name;
+  if (Path.empty())
+return Name;
+
+  for (auto &Piece : Path) {
+if (!Name.empty())
+  Name += ".";
+Name += Piece.first->getName();
+  }
+  return Name;
+}
+
 Sema::DeclGroupPtrTy
 Sema::ActOnGlobalModuleFragmentDecl(SourceLocation ModuleLoc) {
   if (!ModuleScopes.empty() &&
@@ -80,11 +97,10 @@
   return nullptr;
 }
 
-Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation StartLoc,
-   SourceLocation ModuleLoc,
-   ModuleDeclKind MDK,
-   ModuleIdPath Path,
-   ModuleImportState &ImportState) {
+Sema::DeclGroupPtrTy
+Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
+  ModuleDeclKind MDK, ModuleIdPath Path,
+  ModuleIdPath Partition, ModuleImportState &ImportState) {
   assert((getLangOpts().ModulesTS || getLangOpts().CPlusPlusModules) &&
  "should only have module decl in Modules TS or C++20");
 
@@ -163,19 +179,20 @@
   // Flatten the dots in a modul

[PATCH] D116088: [compiler-rt] Implement ARM atomic operations for architectures without SMP support

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Posted D120026  .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116088

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

[PATCH] D114543: Extend the `uwtable` attribute with unwind table kind

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: llvm/lib/IR/Attributes.cpp:453
+return "uwtable";
+  return ("uwtable(" + Twine(Kind == UWTableKind::Sync ? "sync" : "async") 
+
+  ")")

@chill Static analysis is warning that its impossible to hit the if(Kind == 
Default) case here - it looks like you have merged 2 versions of the same (Kind 
!= UWTableKind::None) handling code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114543

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

[PATCH] D120028: [clang-format] Do not add space after return-like keywords in macros.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius created this revision.
curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan.
curdeius requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120028

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1861,6 +1861,16 @@
"#define BBB }\n",
Style);
   // verifyFormat("#define AAA N { //\n", Style);
+
+  verifyFormat("MACRO(return)");
+  verifyFormat("MACRO(co_await)");
+  verifyFormat("MACRO(co_return)");
+  verifyFormat("MACRO(co_yield)");
+  verifyFormat("MACRO(return, something)");
+  verifyFormat("MACRO(co_return, something)");
+  verifyFormat("MACRO(something##something)");
+  verifyFormat("MACRO(return##something)");
+  verifyFormat("MACRO(co_return##something)");
 }
 
 TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2989,7 +2989,8 @@
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
   const FormatToken &Left,
   const FormatToken &Right) {
-  if (Left.is(tok::kw_return) && Right.isNot(tok::semi))
+  if (Left.is(tok::kw_return) &&
+  !Right.isOneOf(tok::semi, tok::r_paren, tok::hashhash))
 return true;
   if (Style.isJson() && Left.is(tok::string_literal) && Right.is(tok::colon))
 return false;
@@ -3026,7 +3027,7 @@
 return false;
   // co_await (x), co_yield (x), co_return (x)
   if (Left.isOneOf(tok::kw_co_await, tok::kw_co_yield, tok::kw_co_return) &&
-  Right.isNot(tok::semi))
+  !Right.isOneOf(tok::semi, tok::r_paren))
 return true;
 
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1861,6 +1861,16 @@
"#define BBB }\n",
Style);
   // verifyFormat("#define AAA N { //\n", Style);
+
+  verifyFormat("MACRO(return)");
+  verifyFormat("MACRO(co_await)");
+  verifyFormat("MACRO(co_return)");
+  verifyFormat("MACRO(co_yield)");
+  verifyFormat("MACRO(return, something)");
+  verifyFormat("MACRO(co_return, something)");
+  verifyFormat("MACRO(something##something)");
+  verifyFormat("MACRO(return##something)");
+  verifyFormat("MACRO(co_return##something)");
 }
 
 TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2989,7 +2989,8 @@
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
   const FormatToken &Left,
   const FormatToken &Right) {
-  if (Left.is(tok::kw_return) && Right.isNot(tok::semi))
+  if (Left.is(tok::kw_return) &&
+  !Right.isOneOf(tok::semi, tok::r_paren, tok::hashhash))
 return true;
   if (Style.isJson() && Left.is(tok::string_literal) && Right.is(tok::colon))
 return false;
@@ -3026,7 +3027,7 @@
 return false;
   // co_await (x), co_yield (x), co_return (x)
   if (Left.isOneOf(tok::kw_co_await, tok::kw_co_yield, tok::kw_co_return) &&
-  Right.isNot(tok::semi))
+  !Right.isOneOf(tok::semi, tok::r_paren))
 return true;
 
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116088: [compiler-rt] Implement ARM atomic operations for architectures without SMP support

[Eli Friedman via Phabricator via cfe-commits]

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

Reverted in 0389f2edf7 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116088

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

[PATCH] D119686: [RISCV] Add the passthru operand for nomask vadc/vsbc/vmerge/vfmerge IR intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG093ecccdab47: [RISCV] Add the passthru operand for 
vadc/vsbc/vmerge/vfmerge IR intrinsics. (authored by khchen).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119686

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vadc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmerge.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmerge.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vsbc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vadc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmerge.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmerge.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vsbc.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
  llvm/test/CodeGen/RISCV/rvv/unmasked-tu.ll
  llvm/test/CodeGen/RISCV/rvv/vadc-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vadc-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vfmerge.ll
  llvm/test/CodeGen/RISCV/rvv/vmerge-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmerge-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vsbc-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vsbc-rv64.ll

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

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

[Iain Sandoe via Phabricator via cfe-commits]

iains marked 7 inline comments as done.
iains added a comment.

some comments remain to be addressed - this update addressed Nathan's primarily 
- but also cover some of Chuanqi's




Comment at: clang/include/clang/Sema/Sema.h:2989
   /// \param ImportLoc The location of the 'import' keyword.
-  /// \param Path The module access path.
+  /// \param NamePath The module toplevel name as an access path.
+  /// \param Partition The module partition name as an access path.

ChuanqiXu wrote:
> urnathan wrote:
> > Is `NamePath` really a better name?  You've not consistently changed all 
> > `Path`'s to this, and it doesn;t strike me as particularly mnemonic.
> I use `Path` in my implementation.
I\ve reverted my changes - leaving the variable as "Path"

This is a problem with the code serving two masters ...

... for clang hierachical modules "Path" makes some sense
... for  C++20 not really - it is not a pathname (confused me when first 
reading the code).

however "NamedModuleNameAsPath" is too long ;)




Comment at: clang/lib/Parse/Parser.cpp:2394
 /// Parse a module import declaration. This is essentially the same for
-/// Objective-C and the C++ Modules TS, except for the leading '@' (in ObjC)
+/// Objective-C and the C++20/Modules TS, except for the leading '@' (in ObjC)
 /// and the trailing optional attributes (in C++).

urnathan wrote:
> perhaps now we should just remove modules-ts references as drive-by cleanups?
will do this generally now .. given consensus amongst active reviewers.



Comment at: clang/lib/Sema/SemaModule.cpp:177
+  if (IsPartition) {
+ModuleName += ":";
+ModuleName += stringFromPath(Partition);

ChuanqiXu wrote:
> I chose '-' in my implementation since here ModuleName refers to the name in 
> filesystem, right? And I remember '-' is the choice of GCC. So let's keep 
> consistency.
This is not my understanding.

ModuleName is the "user-facing" name of the module that is described in the 
source, and we use this in diagnostics etc.

The translation of that name to an on-disk name can be arbitrary and there are 
several mechanisms in clang already (e.g. -fmodule-file=A:B=foo.pcm) the module 
loader uses these to find the module on the basis of its user-facing name where 
required.

When we have P1184, it is even more important that the interface is supplied 
with the name that the user will put into the source code.





Comment at: clang/lib/Sema/SemaModule.cpp:224-225
+if (IsPartition) {
+  // Create an interface, but note that it is an implementation
+  // unit.
   Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName,

ChuanqiXu wrote:
> I prefer to add more words here to tell why we create an interface unit but 
> the above condition is `ModuleDeclKind::Implementation`. 
I am sorry, perhaps 4/8 should have been squashed with this patch - but this 
was getting quite large anyway - please see 4/8 for reorganisation of this code.



Comment at: clang/lib/Sema/SemaModule.cpp:260
+  ModuleScopes.back().ModuleInterface =
+  (MDK != ModuleDeclKind::Implementation || IsPartition);
+  ModuleScopes.back().IsPartition = IsPartition;

ChuanqiXu wrote:
> How about: `Mod->Kind != Module::ModulePartitionImplementation`.
please see 4/8



Comment at: clang/lib/Sema/SemaModule.cpp:346-347
SourceLocation ImportLoc,
-   ModuleIdPath Path) {
-  // Flatten the module path for a C++20 or Modules TS module name.
+   ModuleIdPath NamePath,
+   ModuleIdPath Partition) {
+

ChuanqiXu wrote:
> I think we could change the signature of the lat 2 fields to 
> ```
> ModuleIdPath Path,
> bool IsPartition)
> ```
> I feel this is more simpler.
please see 4/8



Comment at: clang/lib/Sema/SemaModule.cpp:350
+  bool IsPartition = !Partition.empty();
+  bool Cxx20Mode = getLangOpts().CPlusPlusModules || getLangOpts().ModulesTS;
+  assert((!IsPartition || Cxx20Mode) && "partition seen in non-C++20 code?");

ChuanqiXu wrote:
> I prefer to remove the support for getLangOpts().ModulesTS on the fly.
agreed in comments, but I think it is outside of my remit to remove the 
functionality?



Comment at: clang/lib/Sema/SemaModule.cpp:373
+  ModuleName = NamedMod->Name;
+  ModuleName += ":";
 }

ChuanqiXu wrote:
> We could move this below the if statement.
please see 4/8 and later



Comment at: clang/lib/Sema/SemaModule.cpp:477-480
+  } else if (getLangOpts().isCompilingModule()) {
+Module *ThisModule = PP.getHeaderSearchInfo().lookupModule(
+getLangOpts().CurrentModule, ExportLoc, false, false);
+assert(ThisModule && "was expecting a module if building one");
---

[clang] 5065076 - [CodeGen] Rename deprecated Address constructor

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-02-17T11:26:42+01:00
New Revision: 5065076698cf32b5ad3b6f88b5f3b84d68948589

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

LOG: [CodeGen] Rename deprecated Address constructor

To make uses of the deprecated constructor easier to spot, and to
ensure that no new uses are introduced, rename it to
Address::deprecated().

While doing the rename, I've filled in element types in cases
where it was relatively obvious, but we're still left with 135
calls to the deprecated constructor.

Added: 


Modified: 
clang/include/clang/Basic/riscv_vector.td
clang/lib/CodeGen/Address.h
clang/lib/CodeGen/CGAtomic.cpp
clang/lib/CodeGen/CGBlocks.cpp
clang/lib/CodeGen/CGBuilder.h
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CGCUDANV.cpp
clang/lib/CodeGen/CGCXXABI.h
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGClass.cpp
clang/lib/CodeGen/CGCleanup.cpp
clang/lib/CodeGen/CGDecl.cpp
clang/lib/CodeGen/CGException.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprCXX.cpp
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/CodeGen/CGNonTrivialStruct.cpp
clang/lib/CodeGen/CGObjC.cpp
clang/lib/CodeGen/CGObjCGNU.cpp
clang/lib/CodeGen/CGObjCMac.cpp
clang/lib/CodeGen/CGObjCRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/lib/CodeGen/TargetInfo.cpp
clang/utils/TableGen/MveEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/riscv_vector.td 
b/clang/include/clang/Basic/riscv_vector.td
index efc074aba246a..a497f85705c72 100644
--- a/clang/include/clang/Basic/riscv_vector.td
+++ b/clang/include/clang/Basic/riscv_vector.td
@@ -630,7 +630,7 @@ multiclass RVVVLEFFBuiltin types> {
 clang::CharUnits Align =
 CGM.getNaturalPointeeTypeAlignment(E->getArg(1)->getType());
 Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {1}),
-Address(NewVL, Align));
+Address::deprecated(NewVL, Align));
 return V;
   }
   }],
@@ -650,7 +650,7 @@ multiclass RVVVLEFFBuiltin types> {
 clang::CharUnits Align =
 CGM.getNaturalPointeeTypeAlignment(E->getArg(3)->getType());
 Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {1}),
-Address(NewVL, Align));
+Address::deprecated(NewVL, Align));
 return V;
   }
   }] in {
@@ -868,7 +868,7 @@ multiclass RVVUnitStridedSegLoad {
   llvm::Value *V;
   for (unsigned I = 0; I < NF; ++I) {
 V = Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {I}),
-Address(Ops[I], Align));
+Address::deprecated(Ops[I], Align));
   }
   return V;
 }
@@ -894,7 +894,7 @@ multiclass RVVUnitStridedSegLoad {
   llvm::Value *V;
   for (unsigned I = 0; I < NF; ++I) {
 V = Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {I}),
-Address(Ops[I], Align));
+Address::deprecated(Ops[I], Align));
   }
   return V;
 }
@@ -939,11 +939,11 @@ multiclass RVVUnitStridedSegLoadFF {
   CGM.getNaturalPointeeTypeAlignment(E->getArg(0)->getType());
   for (unsigned I = 0; I < NF; ++I) {
 Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {I}),
-Address(Ops[I], Align));
+Address::deprecated(Ops[I], Align));
   }
   // Store new_vl.
   return Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {NF}),
- Address(NewVL, Align));
+ Address::deprecated(NewVL, Align));
 }
 }],
 ManualCodegenMask = [{
@@ -967,11 +967,11 @@ multiclass RVVUnitStridedSegLoadFF {
   CGM.getNaturalPointeeTypeAlignment(E->getArg(0)->getType());
   for (unsigned I = 0; I < NF; ++I) {
 Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {I}),
-Address(Ops[I], Align));
+Address::deprecated(Ops[I], Align));
   }
   // Store new_vl.
   return Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {NF}),
- Address(NewVL, Align));
+ Address::deprecated(NewVL, Align));
 }
 }] in {
   defvar PV = PVString.S;
@@ -1014,

[PATCH] D119599: Add option to align compound assignments like `+=`

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/include/clang/Format/Format.h:163
+  /// \endcode
+  bool AlignCompoundAssignments;
+

sstwcw wrote:
> curdeius wrote:
> > HazardyKnusperkeks wrote:
> > > MyDeveloperDay wrote:
> > > > This option is not independent of `AlignConsecutiveAssignments` is it? 
> > > > this will cause confusion when people turn it on without turning on 
> > > > `AlignConsecutiveAssignments` 
> > > > 
> > > > Options have a lifecycle we have noticed
> > > > 
> > > > 1) They start as bool
> > > > 2) They become enums
> > > > 3) They then end up as a struct of bool
> > > > 4) Sometimes the struct becomes of enums
> > > > 
> > > > Whilst I like what you are doing here I fear we will get bugs where 
> > > > people say I set AlignCompoundAssignments: true but its doesn't work.
> > > > 
> > > > `AlignConsecutiveAssignments` is already gone too far on the enum, it 
> > > > should be a struct
> > > > 
> > > > so rather than
> > > > 
> > > > ```
> > > > enum AlignConsecutiveStyle {
> > > > ACS_None,
> > > > ACS_Consecutive,
> > > > ACS_AcrossEmptyLines,
> > > > ACS_AcrossComments,
> > > > ACS_AcrossEmptyLinesAndComments
> > > >   };
> > > > AlignConsecutiveStyle  AlignConsecutiveAssignments ;
> > > > 
> > > > ```
> > > > it should be perhaps
> > > > 
> > > > ```
> > > > struct {
> > > > bool AcrossEmptyLines,
> > > > bool AcrossComments,
> > > > bool AlignCompound
> > > > } AlignConsecutiveStyle;
> > > > 
> > > > AlignConsecutiveStyle  AlignConsecutiveAssignments;
> > > > ```
> > > > 
> > > > in the .clang-format file it would become
> > > > 
> > > > ```
> > > > AlignConsecutiveAssignments: Custom
> > > > AlignConsecutiveAssignmentsOptions:
> > > >AcrossEmptyLines: true
> > > >AcrossComments: true
> > > >AlignCompound: false
> > > > ```
> > > > 
> > > > I realise this would be a much bigger piece of work (in the config) but 
> > > > the existing options would map to the new options, and then we have a 
> > > > structure for which have space for future expansion.
> > > > 
> > > > The introduction of a dependent option in my view triggers the need for 
> > > > that config change? @HazardyKnusperkeks 
> > > >  you thoughts, I know we've done this before, what  do you think in 
> > > > this case? 
> > > > 
> > > > 
> > > > 
> > > I would even go further (and that I already told the last time). Drop the 
> > > ``Custom`` and map the old enums to the struct when parsing, so no new 
> > > option.
> > > I would even go further (and that I already told the last time). Drop the 
> > > ``Custom`` and map the old enums to the struct when parsing, so no new 
> > > option.
> > 
> > :+1:
> > 
> > That's my preference too. Having both `AlignConsecutiveAssignments` and 
> > `AlignConsecutiveAssignmentsOptions` is error-prone.
> About `AlignConsecutiveAssignments` and `AlignConsecutiveAssignmentsOptions`. 
> Based on the current YAML infrastructure, it does not seem possible to 
> support both enum and struct under one name.
Grrr, indeed, that doesn't seem easy. I'm gonna play a bit more with 
`yaml::PolymorphicTraits` but not sure it's of any help here.
So yeah, please go on with this revision as if I weren't doing anything :).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D119012: [flang][driver] Add support for the `-emit-llvm` option

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

A believe that all of Kiran's comments have been addressed and pre-merge CI is 
also passing now (that was one of the concerns). Kiran has not accepted this 
yet, but he will be away for a few weeks. I will merge this as is to unblock 
further work. I more than happy to address any post-commit comments, Kiran!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119012

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

[PATCH] D114543: Extend the `uwtable` attribute with unwind table kind

[Momchil Velikov via Phabricator via cfe-commits]

chill added inline comments.



Comment at: llvm/lib/IR/Attributes.cpp:453
+return "uwtable";
+  return ("uwtable(" + Twine(Kind == UWTableKind::Sync ? "sync" : "async") 
+
+  ")")

RKSimon wrote:
> @chill Static analysis is warning that its impossible to hit the if(Kind == 
> Default) case here - it looks like you have merged 2 versions of the same 
> (Kind != UWTableKind::None) handling code?
Thanks, indeed.

Fixed in https://reviews.llvm.org/D120030


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114543

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

[clang] 9798b33 - [OpenCL] Guard 64-bit atomic types

[Sven van Haastregt via cfe-commits]

Author: Sven van Haastregt
Date: 2022-02-17T10:58:52Z
New Revision: 9798b33d1dc14f5334e2cc117e3896510fa57b82

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

LOG: [OpenCL] Guard 64-bit atomic types

Until now, overloads with a 64-bit atomic type argument were always
made available with `-fdeclare-opencl-builtins`.  Ensure these
overloads are only available when both the `cl_khr_int64_base_atomics`
and `cl_khr_int64_extended_atomics` extensions have been enabled, as
required by the OpenCL specification.

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

Added: 


Modified: 
clang/lib/Sema/OpenCLBuiltins.td
clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Removed: 




diff  --git a/clang/lib/Sema/OpenCLBuiltins.td 
b/clang/lib/Sema/OpenCLBuiltins.td
index 556d1778625e7..e6da5e34f7091 100644
--- a/clang/lib/Sema/OpenCLBuiltins.td
+++ b/clang/lib/Sema/OpenCLBuiltins.td
@@ -78,6 +78,8 @@ class concatExtension 
{
 def NoTypeExt   : TypeExtension<"">;
 def Fp16TypeExt : TypeExtension<"cl_khr_fp16">;
 def Fp64TypeExt : TypeExtension<"cl_khr_fp64">;
+def Atomic64TypeExt : TypeExtension<"cl_khr_int64_base_atomics 
cl_khr_int64_extended_atomics">;
+def AtomicFp64TypeExt : TypeExtension<"cl_khr_int64_base_atomics 
cl_khr_int64_extended_atomics cl_khr_fp64">;
 
 // FunctionExtension definitions.
 def FuncExtNone  : FunctionExtension<"">;
@@ -389,10 +391,14 @@ def NDRange   : TypedefType<"ndrange_t">;
 // OpenCL v2.0 s6.13.11: Atomic integer and floating-point types.
 def AtomicInt : Type<"atomic_int", 
QualType<"Context.getAtomicType(Context.IntTy)">>;
 def AtomicUInt: Type<"atomic_uint", 
QualType<"Context.getAtomicType(Context.UnsignedIntTy)">>;
-def AtomicLong: Type<"atomic_long", 
QualType<"Context.getAtomicType(Context.LongTy)">>;
-def AtomicULong   : Type<"atomic_ulong", 
QualType<"Context.getAtomicType(Context.UnsignedLongTy)">>;
+let Extension = Atomic64TypeExt in {
+  def AtomicLong: Type<"atomic_long", 
QualType<"Context.getAtomicType(Context.LongTy)">>;
+  def AtomicULong   : Type<"atomic_ulong", 
QualType<"Context.getAtomicType(Context.UnsignedLongTy)">>;
+}
 def AtomicFloat   : Type<"atomic_float", 
QualType<"Context.getAtomicType(Context.FloatTy)">>;
-def AtomicDouble  : Type<"atomic_double", 
QualType<"Context.getAtomicType(Context.DoubleTy)">>;
+let Extension = AtomicFp64TypeExt in {
+  def AtomicDouble  : Type<"atomic_double", 
QualType<"Context.getAtomicType(Context.DoubleTy)">>;
+}
 def AtomicHalf: Type<"atomic_half", 
QualType<"Context.getAtomicType(Context.HalfTy)">>;
 def AtomicIntPtr  : Type<"atomic_intptr_t", 
QualType<"Context.getAtomicType(Context.getIntPtrType())">>;
 def AtomicUIntPtr : Type<"atomic_uintptr_t", 
QualType<"Context.getAtomicType(Context.getUIntPtrType())">>;

diff  --git a/clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl 
b/clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
index d526c32d65a92..d2d7fff02efaa 100644
--- a/clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ b/clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -163,6 +163,25 @@ void test_atomic_fetch_with_address_space(volatile 
__generic atomic_float *a_flo
 }
 #endif // !defined(NO_HEADER) && __OPENCL_C_VERSION__ >= 200
 
+#if !defined(NO_HEADER) && __OPENCL_C_VERSION__ == 200 && 
defined(__opencl_c_generic_address_space)
+
+// Test that overloads that use atomic_double are not available when the fp64
+// extension is disabled.  Test this by counting the number of notes about
+// candidate functions.
+void test_atomic_double_reporting(volatile __generic atomic_int *a) {
+  atomic_init(a);
+  // expected-error@-1{{no matching function for call to 'atomic_init'}}
+#if defined(NO_FP64)
+  // Expecting 5 candidates: int, uint, long, ulong, float
+  // expected-note@-4 5 {{candidate function not viable: requires 2 arguments, 
but 1 was provided}}
+#else
+  // Expecting 6 candidates: int, uint, long, ulong, float, double
+  // expected-note@-7 6 {{candidate function not viable: requires 2 arguments, 
but 1 was provided}}
+#endif
+}
+
+#endif
+
 #if defined(NO_ATOMSCOPE) && __OPENCL_C_VERSION__ >= 300
 // Disable the feature by undefining the feature macro.
 #undef __opencl_c_atomic_scope_device

diff  --git a/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp 
b/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
index 4795b008dda3c..34ca6cb36738c 100644
--- a/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
+++ b/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -733,6 +733,20 @@ static std::pair 
isOpenCLBuiltin(llvm::StringRef Name) {
   OS << "} // isOpenCLB

[PATCH] D119858: [OpenCL] Guard 64-bit atomic types

[Sven van Haastregt via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9798b33d1dc1: [OpenCL] Guard 64-bit atomic types (authored 
by svenvh).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119858

Files:
  clang/lib/Sema/OpenCLBuiltins.td
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
  clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Index: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
===
--- clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
+++ clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -733,6 +733,20 @@
   OS << "} // isOpenCLBuiltin\n";
 }
 
+// Emit an if-statement with an isMacroDefined call for each extension in
+// the space-separated list of extensions.
+static void EmitMacroChecks(raw_ostream &OS, StringRef Extensions) {
+  SmallVector ExtVec;
+  Extensions.split(ExtVec, " ");
+  OS << "  if (";
+  for (StringRef Ext : ExtVec) {
+if (Ext != ExtVec.front())
+  OS << " && ";
+OS << "S.getPreprocessor().isMacroDefined(\"" << Ext << "\")";
+  }
+  OS << ") {\n  ";
+}
+
 void BuiltinNameEmitter::EmitQualTypeFinder() {
   OS << R"(
 
@@ -825,15 +839,14 @@
 // Collect all QualTypes for a single vector size into TypeList.
 OS << "  SmallVector TypeList;\n";
 for (const auto *T : BaseTypes) {
-  StringRef Ext =
+  StringRef Exts =
   T->getValueAsDef("Extension")->getValueAsString("ExtName");
-  if (!Ext.empty()) {
-OS << "  if (S.getPreprocessor().isMacroDefined(\"" << Ext
-   << "\")) {\n  ";
+  if (!Exts.empty()) {
+EmitMacroChecks(OS, Exts);
   }
   OS << "  TypeList.push_back("
  << T->getValueAsDef("QTExpr")->getValueAsString("TypeExpr") << ");\n";
-  if (!Ext.empty()) {
+  if (!Exts.empty()) {
 OS << "  }\n";
   }
 }
@@ -877,15 +890,14 @@
 // Emit the cases for non generic, non image types.
 OS << "case OCLT_" << T->getValueAsString("Name") << ":\n";
 
-StringRef Ext = T->getValueAsDef("Extension")->getValueAsString("ExtName");
-// If this type depends on an extension, ensure the extension macro is
+StringRef Exts = T->getValueAsDef("Extension")->getValueAsString("ExtName");
+// If this type depends on an extension, ensure the extension macros are
 // defined.
-if (!Ext.empty()) {
-  OS << "  if (S.getPreprocessor().isMacroDefined(\"" << Ext
- << "\")) {\n  ";
+if (!Exts.empty()) {
+  EmitMacroChecks(OS, Exts);
 }
 OS << "  QT.push_back(" << QT->getValueAsString("TypeExpr") << ");\n";
-if (!Ext.empty()) {
+if (!Exts.empty()) {
   OS << "  }\n";
 }
 OS << "  break;\n";
Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -163,6 +163,25 @@
 }
 #endif // !defined(NO_HEADER) && __OPENCL_C_VERSION__ >= 200
 
+#if !defined(NO_HEADER) && __OPENCL_C_VERSION__ == 200 && defined(__opencl_c_generic_address_space)
+
+// Test that overloads that use atomic_double are not available when the fp64
+// extension is disabled.  Test this by counting the number of notes about
+// candidate functions.
+void test_atomic_double_reporting(volatile __generic atomic_int *a) {
+  atomic_init(a);
+  // expected-error@-1{{no matching function for call to 'atomic_init'}}
+#if defined(NO_FP64)
+  // Expecting 5 candidates: int, uint, long, ulong, float
+  // expected-note@-4 5 {{candidate function not viable: requires 2 arguments, but 1 was provided}}
+#else
+  // Expecting 6 candidates: int, uint, long, ulong, float, double
+  // expected-note@-7 6 {{candidate function not viable: requires 2 arguments, but 1 was provided}}
+#endif
+}
+
+#endif
+
 #if defined(NO_ATOMSCOPE) && __OPENCL_C_VERSION__ >= 300
 // Disable the feature by undefining the feature macro.
 #undef __opencl_c_atomic_scope_device
Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -78,6 +78,8 @@
 def NoTypeExt   : TypeExtension<"">;
 def Fp16TypeExt : TypeExtension<"cl_khr_fp16">;
 def Fp64TypeExt : TypeExtension<"cl_khr_fp64">;
+def Atomic64TypeExt : TypeExtension<"cl_khr_int64_base_atomics cl_khr_int64_extended_atomics">;
+def AtomicFp64TypeExt : TypeExtension<"cl_khr_int64_base_atomics cl_khr_int64_extended_atomics cl_khr_fp64">;
 
 // FunctionExtension definitions.
 def FuncExtNone  : FunctionExtension<"">;
@@ -389,10 +391,14 @@
 // OpenCL v2.0 s6.13.11: Atomic integer and floating-point types.
 def AtomicInt : Type<"atomic_int", Qual

[PATCH] D120032: [OpenCL] opencl-c.h: use uint/ulong consistently

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh created this revision.
svenvh added a reviewer: Anastasia.
svenvh added a project: clang.
Herald added subscribers: Naghasan, ldrumm, yaxunl.
svenvh requested review of this revision.
Herald added a subscriber: cfe-commits.

Most places already seem to use the short spelling instead of
'unsigned int/long', so perform the following substitutions:

  s/unsigned int /uint /g
  s/unsigned long /ulong /g

This simplifies completeness comparisons against OpenCLBuiltins.td.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120032

Files:
  clang/lib/Headers/opencl-c.h

Index: clang/lib/Headers/opencl-c.h
===
--- clang/lib/Headers/opencl-c.h
+++ clang/lib/Headers/opencl-c.h
@@ -12919,28 +12919,28 @@
  * pointed by p. The function returns old.
  */
 int __ovld atomic_add(volatile __global int *p, int val);
-unsigned int __ovld atomic_add(volatile __global unsigned int *p, unsigned int val);
+uint __ovld atomic_add(volatile __global uint *p, uint val);
 int __ovld atomic_add(volatile __local int *p, int val);
-unsigned int __ovld atomic_add(volatile __local unsigned int *p, unsigned int val);
+uint __ovld atomic_add(volatile __local uint *p, uint val);
 #ifdef __OPENCL_CPP_VERSION__
 int __ovld atomic_add(volatile int *p, int val);
-unsigned int __ovld atomic_add(volatile unsigned int *p, unsigned int val);
+uint __ovld atomic_add(volatile uint *p, uint val);
 #endif
 
 #if defined(cl_khr_global_int32_base_atomics)
 int __ovld atom_add(volatile __global int *p, int val);
-unsigned int __ovld atom_add(volatile __global unsigned int *p, unsigned int val);
+uint __ovld atom_add(volatile __global uint *p, uint val);
 #endif
 #if defined(cl_khr_local_int32_base_atomics)
 int __ovld atom_add(volatile __local int *p, int val);
-unsigned int __ovld atom_add(volatile __local unsigned int *p, unsigned int val);
+uint __ovld atom_add(volatile __local uint *p, uint val);
 #endif
 
 #if defined(cl_khr_int64_base_atomics)
 long __ovld atom_add(volatile __global long *p, long val);
-unsigned long __ovld atom_add(volatile __global unsigned long *p, unsigned long val);
+ulong __ovld atom_add(volatile __global ulong *p, ulong val);
 long __ovld atom_add(volatile __local long *p, long val);
-unsigned long __ovld atom_add(volatile __local unsigned long *p, unsigned long val);
+ulong __ovld atom_add(volatile __local ulong *p, ulong val);
 #endif
 
 /**
@@ -12949,28 +12949,28 @@
  * returns old.
  */
 int __ovld atomic_sub(volatile __global int *p, int val);
-unsigned int __ovld atomic_sub(volatile __global unsigned int *p, unsigned int val);
+uint __ovld atomic_sub(volatile __global uint *p, uint val);
 int __ovld atomic_sub(volatile __local int *p, int val);
-unsigned int __ovld atomic_sub(volatile __local unsigned int *p, unsigned int val);
+uint __ovld atomic_sub(volatile __local uint *p, uint val);
 #ifdef __OPENCL_CPP_VERSION__
 int __ovld atomic_sub(volatile int *p, int val);
-unsigned int __ovld atomic_sub(volatile unsigned int *p, unsigned int val);
+uint __ovld atomic_sub(volatile uint *p, uint val);
 #endif
 
 #if defined(cl_khr_global_int32_base_atomics)
 int __ovld atom_sub(volatile __global int *p, int val);
-unsigned int __ovld atom_sub(volatile __global unsigned int *p, unsigned int val);
+uint __ovld atom_sub(volatile __global uint *p, uint val);
 #endif
 #if defined(cl_khr_local_int32_base_atomics)
 int __ovld atom_sub(volatile __local int *p, int val);
-unsigned int __ovld atom_sub(volatile __local unsigned int *p, unsigned int val);
+uint __ovld atom_sub(volatile __local uint *p, uint val);
 #endif
 
 #if defined(cl_khr_int64_base_atomics)
 long __ovld atom_sub(volatile __global long *p, long val);
-unsigned long __ovld atom_sub(volatile __global unsigned long *p, unsigned long val);
+ulong __ovld atom_sub(volatile __global ulong *p, ulong val);
 long __ovld atom_sub(volatile __local long *p, long val);
-unsigned long __ovld atom_sub(volatile __local unsigned long *p, unsigned long val);
+ulong __ovld atom_sub(volatile __local ulong *p, ulong val);
 #endif
 
 /**
@@ -12979,31 +12979,31 @@
  * value.
  */
 int __ovld atomic_xchg(volatile __global int *p, int val);
-unsigned int __ovld atomic_xchg(volatile __global unsigned int *p, unsigned int val);
+uint __ovld atomic_xchg(volatile __global uint *p, uint val);
 int __ovld atomic_xchg(volatile __local int *p, int val);
-unsigned int __ovld atomic_xchg(volatile __local unsigned int *p, unsigned int val);
+uint __ovld atomic_xchg(volatile __local uint *p, uint val);
 float __ovld atomic_xchg(volatile __global float *p, float val);
 float __ovld atomic_xchg(volatile __local float *p, float val);
 #ifdef __OPENCL_CPP_VERSION__
 int __ovld atomic_xchg(volatile int *p, int val);
-unsigned int __ovld atomic_xchg(volatile unsigned int *p, unsigned int val);
+uint __ovld atomic_xchg(volatile uint *p, uint val);
 float __ovld atomic_xchg(volatile float *p, float val);
 #endif
 
 #if defined(c

[PATCH] D120034: [clang-format] PROPOSAL - WIP: Added ability to parse template arguments

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

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, curdeius, MyDeveloperDay.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Obviously not implemented yet.

For https://github.com/llvm/llvm-project/issues/53864 I need to get the end of 
the requires clause at the end of the nested template. But `parseBracedList()` 
just ends on the first `>` it finds.

Simply extending it the be recursive on `<` wouldn't work, since we have to 
determine if `<` is a `less` or a template opener. Analogous for `>`. Currently 
we have `TokenAnnotator::parseAngle` to deal with this stuff. Including `// 
FIXME: This is getting out of hand, write a decent parser.`

So my proposal is to add this parsing in `UnwrappedLineParser` and set 
`TT_TemplateOpener` and `TT_TempalteCloser` already there.

This is going to be a huge change, so I ask for your opinion before I put a lot 
of effort into that.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120034

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h


Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -118,6 +118,7 @@
tok::TokenKind ClosingBraceKind = tok::r_brace);
   void parseParens(TokenType AmpAmpTokenType = TT_Unknown);
   void parseSquare(bool LambdaIntroducer = false);
+  void parseTemplateArguments();
   void keepAncestorBraces();
   FormatToken *parseIfThenElse(IfStmtKind *IfKind, bool KeepBraces = false);
   void parseTryCatch();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2289,6 +2289,21 @@
   } while (!eof());
 }
 
+/// \brief Parses a template arguments, that is from the opening < to the
+/// closing >. Returns when it succesfully read the template arguments or
+/// detected that it's not a template argument list.
+void clang::format::UnwrappedLineParser::parseTemplateArguments() {
+  assert(FormatTok->is(tok::less) && "'<' expected.");
+  assert(FormatTok->is(TT_Unknown) && "Type already set on '<'");
+  auto OpeningAngle = FormatTok;
+  nextToken();
+
+
+  do {
+//Do stuff
+  } while (!eof());
+}
+
 void UnwrappedLineParser::keepAncestorBraces() {
   if (!Style.RemoveBracesLLVM)
 return;


Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -118,6 +118,7 @@
tok::TokenKind ClosingBraceKind = tok::r_brace);
   void parseParens(TokenType AmpAmpTokenType = TT_Unknown);
   void parseSquare(bool LambdaIntroducer = false);
+  void parseTemplateArguments();
   void keepAncestorBraces();
   FormatToken *parseIfThenElse(IfStmtKind *IfKind, bool KeepBraces = false);
   void parseTryCatch();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2289,6 +2289,21 @@
   } while (!eof());
 }
 
+/// \brief Parses a template arguments, that is from the opening < to the
+/// closing >. Returns when it succesfully read the template arguments or
+/// detected that it's not a template argument list.
+void clang::format::UnwrappedLineParser::parseTemplateArguments() {
+  assert(FormatTok->is(tok::less) && "'<' expected.");
+  assert(FormatTok->is(TT_Unknown) && "Type already set on '<'");
+  auto OpeningAngle = FormatTok;
+  nextToken();
+
+
+  do {
+//Do stuff
+  } while (!eof());
+}
+
 void UnwrappedLineParser::keepAncestorBraces() {
   if (!Style.RemoveBracesLLVM)
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119375: [Clang][Sema] Do not evaluate value-dependent immediate invocations

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added a comment.

A friendly ping =) Seems like I don't have write access, so unfortunately I 
have to ask people to merge commits on my behalf. Let me copy-paste the usual 
comment of my reviews:

//P.S. If this review is eventually approved, kindly please merge the commit on 
my behalf =) As I don't have merge access. My name is `Evgeny Shulgin` and 
email is `izaronpl...@gmail.com`. Sorry for inconvenience!//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119375

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

[PATCH] D119409: [C++20] [Modules] Remain internal-linkage variables in module interface unit

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added inline comments.



Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5
 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global
-// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global
+// CHECK-DAG: @inline_var_exported = available_externally {{(dso_local 
)?}}global
 // CHECK-DAG: @const_var_exported = available_externally {{(dso_local 
)?}}constant i32 3,

ChuanqiXu wrote:
> urnathan wrote:
> > ChuanqiXu wrote:
> > > dblaikie wrote:
> > > > ChuanqiXu wrote:
> > > > > ChuanqiXu wrote:
> > > > > > urnathan wrote:
> > > > > > > ChuanqiXu wrote:
> > > > > > > > urnathan wrote:
> > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > urnathan wrote:
> > > > > > > > > > > I don;t think this is correct.  That should still be a 
> > > > > > > > > > > linkonce odr, otherwise you'll get conflicts with other 
> > > > > > > > > > > module implementation units.
> > > > > > > > > > It is still linkonce_odr in the module it get defined. See 
> > > > > > > > > > the new added test case: inline-variable-in-module.cpp for 
> > > > > > > > > > example. The attribute `available_externally` is equivalent 
> > > > > > > > > > to external from the perspective of linker. See 
> > > > > > > > > > https://llvm.org/docs/LangRef.html#linkage-types. According 
> > > > > > > > > > to [dcl.inline]p7, inline variable attached to named module 
> > > > > > > > > > should be defined in that domain. Note that the variable 
> > > > > > > > > > attached to global module fragment and private module 
> > > > > > > > > > fragment shouldn't be accessed outside the module, so it 
> > > > > > > > > > implies that all the variable defined in the module could 
> > > > > > > > > > only be defined in the module unit itself.
> > > > > > > > > There's a couple of issues with this.  module.cppm is 
> > > > > > > > > emitting a (linkonce) definition of inlne_var_exported, but 
> > > > > > > > > only because it itself is ODR-using that variable.  If you 
> > > > > > > > > take out the ODR-use in noninline_exported, there is no 
> > > > > > > > > longer a symbol emitted.
> > > > > > > > > 
> > > > > > > > > But, even if you forced inline vars to be emitted in their 
> > > > > > > > > defining-module's interface unit, that would be an ABI 
> > > > > > > > > change.  inline vars are emitted whereever ODR-used.  They 
> > > > > > > > > have no fixed home TU.  Now, we could alter the ABI and allow 
> > > > > > > > > interface units to define a home location for inline vars and 
> > > > > > > > > similar entities (eg, vtables for keyless classes).  But we'd 
> > > > > > > > > need buy-in from other compilers to do that.
> > > > > > > > > 
> > > > > > > > > FWIW such a discussion did come up early in implementing 
> > > > > > > > > modules-ts, but we decided there was enough going on just 
> > > > > > > > > getting the TS implemented.  I'm fine with revisiting that, 
> > > > > > > > > but it is a more significant change.
> > > > > > > > > 
> > > > > > > > > And it wouldn't apply to (eg) templated variables, which of 
> > > > > > > > > course could be instantiated anywhere.
> > > > > > > > Oh, now the key point here is what the correct behavior is 
> > > > > > > > instead of the patch. Let's discuss it first.
> > > > > > > > 
> > > > > > > > According to [[ http://eel.is/c++draft/dcl.inline#7 | 
> > > > > > > > [dcl.inline]p7 ]], 
> > > > > > > > > If an inline function or variable that is attached to a named 
> > > > > > > > > module is declared in a definition domain, it shall be 
> > > > > > > > > defined in that domain.
> > > > > > > > 
> > > > > > > > I think the intention of the sentence is to define inline 
> > > > > > > > variable in the module interface. So if it is required by the 
> > > > > > > > standard, I think other compiler need to follow up. As I 
> > > > > > > > described in the summary, it might be a difference between 
> > > > > > > > C++20 module and ModuleTS. Do you think it is necessary to send 
> > > > > > > > the question to WG21? (I get the behavior from reading the 
> > > > > > > > words. Maybe I misread or the word is not intentional).
> > > > > > > > 
> > > > > > > > Maybe the ABI standard group need to discuss what the linkage 
> > > > > > > > should be. Now it may be weak_odr or linkonce_odr. It depends 
> > > > > > > > on how we compile the file. If we compile the .cppm file 
> > > > > > > > directly, it would be linkonce_odr. And if we compile it to 
> > > > > > > > *.pcm file first, it would be weak_odr. I have registered an 
> > > > > > > > issue for this: 
> > > > > > > > https://github.com/llvm/llvm-project/issues/53838.
> > > > > > > > Oh, now the key point here is what the correct behavior is 
> > > > > > > > instead of the patch. Let's discuss it first.
> > > > > > > > 
> > > > > > > > According to [[ http://eel.is/c++draft/dcl.inline#7 | 
> > > > > > > > [dcl.inline]p7 ]], 
> > > > > > > > > If an inline function or variable

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682
+Expr::EvalResult EVRX, EVRY;
+if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) ||
+!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C))
+  return false;
+
+APValue VX = EVRX.Val, VY = EVRY.Val;
+if (VX.getKind() != VY.getKind())

ChuanqiXu wrote:
> urnathan wrote:
> > I'm kind of surprised how complex this check is.  Isn't there an AST 
> > comparator available somewhere?
> I found ODRHash. I think it is much better now.
hm that suggests there there must be a comparator too -- this isn't a 
cryptographically strong hash is it?  How would the compiler currently make use 
of 'definitely different' and 'probably the same' without such a comparator?


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

https://reviews.llvm.org/D118034

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

[clang] e993b20 - [flang][driver] Add support for `-emit-llvm`

[Andrzej Warzynski via cfe-commits]

Author: Andrzej Warzynski
Date: 2022-02-17T12:13:03Z
New Revision: e993b20c049d2f933831c26139f024e022f3d7fe

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

LOG: [flang][driver] Add support for `-emit-llvm`

This patch adds support for the `-emit-llvm` option in the frontend
driver (i.e. `flang-new -fc1`). Similarly to Clang, `flang-new -fc1
-emit-llvm file.f` will generate a textual LLVM IR file.

Depends on D118985

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

Added: 
flang/test/Driver/emit-llvm.f90

Modified: 
clang/include/clang/Driver/Options.td
flang/include/flang/Frontend/FrontendActions.h
flang/include/flang/Frontend/FrontendOptions.h
flang/lib/Frontend/CompilerInvocation.cpp
flang/lib/Frontend/FrontendActions.cpp
flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
flang/test/Driver/driver-help.f90
flang/unittests/Frontend/FrontendActionTest.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index b81973155cae6..37a8e9b77bbfb 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1071,7 +1071,7 @@ def d_Flag : Flag<["-"], "d">, Group;
 def d_Joined : Joined<["-"], "d">, Group;
 def emit_ast : Flag<["-"], "emit-ast">,
   HelpText<"Emit Clang AST files for source inputs">;
-def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option]>, 
Group,
+def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option, FC1Option]>, 
Group,
   HelpText<"Use the LLVM representation for assembler and object files">;
 def emit_interface_stubs : Flag<["-"], "emit-interface-stubs">, 
Flags<[CC1Option]>, Group,
   HelpText<"Generate Interface Stub Files.">;

diff  --git a/flang/include/flang/Frontend/FrontendActions.h 
b/flang/include/flang/Frontend/FrontendActions.h
index e3def74e0f417..6a9afd1afc5c0 100644
--- a/flang/include/flang/Frontend/FrontendActions.h
+++ b/flang/include/flang/Frontend/FrontendActions.h
@@ -13,6 +13,7 @@
 #include "flang/Semantics/semantics.h"
 
 #include "mlir/IR/BuiltinOps.h"
+#include "llvm/IR/Module.h"
 #include 
 
 namespace Fortran::frontend {
@@ -163,12 +164,25 @@ class CodeGenAction : public FrontendAction {
   std::unique_ptr mlirModule;
   std::unique_ptr mlirCtx;
   /// }
+
+  /// @name LLVM IR
+  std::unique_ptr llvmCtx;
+  std::unique_ptr llvmModule;
+
+  /// Generates an LLVM IR module from CodeGenAction::mlirModule and saves it
+  /// in CodeGenAction::llvmModule.
+  void GenerateLLVMIR();
+  /// }
 };
 
 class EmitMLIRAction : public CodeGenAction {
   void ExecuteAction() override;
 };
 
+class EmitLLVMAction : public CodeGenAction {
+  void ExecuteAction() override;
+};
+
 class EmitObjAction : public CodeGenAction {
   void ExecuteAction() override;
 };

diff  --git a/flang/include/flang/Frontend/FrontendOptions.h 
b/flang/include/flang/Frontend/FrontendOptions.h
index 0ff8d0a758873..060910e3d67cd 100644
--- a/flang/include/flang/Frontend/FrontendOptions.h
+++ b/flang/include/flang/Frontend/FrontendOptions.h
@@ -34,6 +34,9 @@ enum ActionKind {
   /// Emit a .mlir file
   EmitMLIR,
 
+  /// Emit an .ll file
+  EmitLLVM,
+
   /// Emit a .o file.
   EmitObj,
 
@@ -84,9 +87,6 @@ enum ActionKind {
 
   /// Run a plugin action
   PluginAction
-
-  /// TODO: RunPreprocessor, EmitLLVM, EmitLLVMOnly,
-  /// EmitCodeGenOnly, EmitAssembly, (...)
 };
 
 /// \param suffix The file extension

diff  --git a/flang/lib/Frontend/CompilerInvocation.cpp 
b/flang/lib/Frontend/CompilerInvocation.cpp
index af59cb6636b3a..7507b0091e13c 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -137,6 +137,9 @@ static bool ParseFrontendArgs(FrontendOptions &opts, 
llvm::opt::ArgList &args,
 case clang::driver::options::OPT_emit_mlir:
   opts.programAction = EmitMLIR;
   break;
+case clang::driver::options::OPT_emit_llvm:
+  opts.programAction = EmitLLVM;
+  break;
 case clang::driver::options::OPT_emit_obj:
   opts.programAction = EmitObj;
   break;

diff  --git a/flang/lib/Frontend/FrontendActions.cpp 
b/flang/lib/Frontend/FrontendActions.cpp
index d981faaa84980..43ab3f689522d 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -14,6 +14,7 @@
 #include "flang/Lower/Bridge.h"
 #include "flang/Lower/PFTBuilder.h"
 #include "flang/Lower/Support/Verifier.h"
+#include "flang/Optimizer/Support/FIRContext.h"
 #include "flang/Optimizer/Support/InitFIR.h"
 #include "flang/Optimizer/Support/KindMapping.h"
 #include "flang/Optimizer/Support/Utils.h"
@@ -28,6 +29,7 @@
 
 #include "mlir/IR/Dialect.h"
 #include "mlir/Pass/PassManager.h"
+#include "mlir/Target/LLVMIR/ModuleTranslation.h"
 #include "llvm/ADT/StringRef

[PATCH] D119012: [flang][driver] Add support for the `-emit-llvm` option

[Andrzej Warzynski via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe993b20c049d: [flang][driver] Add support for `-emit-llvm` 
(authored by awarzynski).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119012

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/FrontendActions.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/driver-help.f90
  flang/test/Driver/emit-llvm.f90
  flang/unittests/Frontend/FrontendActionTest.cpp

Index: flang/unittests/Frontend/FrontendActionTest.cpp
===
--- flang/unittests/Frontend/FrontendActionTest.cpp
+++ flang/unittests/Frontend/FrontendActionTest.cpp
@@ -161,4 +161,31 @@
   .contains(
   ":1:14: error: IF statement is not allowed in IF statement\n"));
 }
+
+TEST_F(FrontendActionTest, EmitLLVM) {
+  // Populate the input file with the pre-defined input and flush it.
+  *(inputFileOs_) << "end program";
+  inputFileOs_.reset();
+
+  // Set-up the action kind.
+  compInst_.invocation().frontendOpts().programAction = EmitLLVM;
+  compInst_.invocation().preprocessorOpts().noReformat = true;
+
+  // Set-up the output stream. We are using output buffer wrapped as an output
+  // stream, as opposed to an actual file (or a file descriptor).
+  llvm::SmallVector outputFileBuffer;
+  std::unique_ptr outputFileStream(
+  new llvm::raw_svector_ostream(outputFileBuffer));
+  compInst_.set_outputStream(std::move(outputFileStream));
+
+  // Execute the action.
+  bool success = ExecuteCompilerInvocation(&compInst_);
+
+  // Validate the expected output.
+  EXPECT_TRUE(success);
+  EXPECT_TRUE(!outputFileBuffer.empty());
+
+  EXPECT_TRUE(llvm::StringRef(outputFileBuffer.data())
+  .contains("define void @_QQmain()"));
+}
 } // namespace
Index: flang/test/Driver/emit-llvm.f90
===
--- /dev/null
+++ flang/test/Driver/emit-llvm.f90
@@ -0,0 +1,22 @@
+! Test the `-emit-llvm` option
+
+! UNSUPPORTED: system-windows
+! Windows is currently not supported in flang/lib/Optimizer/CodeGen/Target.cpp
+
+!
+! RUN COMMAND
+!
+! RUN: %flang_fc1 -emit-llvm %s -o - | FileCheck %s
+
+!
+! EXPECTED OUTPUT
+!
+! CHECK: ; ModuleID = 'FIRModule'
+! CHECK: define void @_QQmain()
+! CHECK-NEXT:  ret void
+! CHECK-NEXT: }
+
+!--
+! INPUT
+!--
+end program
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -65,6 +65,7 @@
 ! HELP-FC1-NEXT:OPTIONS:
 ! HELP-FC1-NEXT: -cpp   Enable predefined and command line preprocessor macros
 ! HELP-FC1-NEXT: -D = Define  to  (or 1 if  omitted)
+! HELP-FC1-NEXT: -emit-llvm Use the LLVM representation for assembler and object files
 ! HELP-FC1-NEXT: -emit-mlir Build the parse tree, then lower it to MLIR
 ! HELP-FC1-NEXT: -emit-obj Emit native object files
 ! HELP-FC1-NEXT: -E Only run the preprocessor
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -35,6 +35,8 @@
 return std::make_unique();
   case EmitMLIR:
 return std::make_unique();
+  case EmitLLVM:
+return std::make_unique();
   case EmitObj:
 return std::make_unique();
   case DebugUnparse:
Index: flang/lib/Frontend/FrontendActions.cpp
===
--- flang/lib/Frontend/FrontendActions.cpp
+++ flang/lib/Frontend/FrontendActions.cpp
@@ -14,6 +14,7 @@
 #include "flang/Lower/Bridge.h"
 #include "flang/Lower/PFTBuilder.h"
 #include "flang/Lower/Support/Verifier.h"
+#include "flang/Optimizer/Support/FIRContext.h"
 #include "flang/Optimizer/Support/InitFIR.h"
 #include "flang/Optimizer/Support/KindMapping.h"
 #include "flang/Optimizer/Support/Utils.h"
@@ -28,6 +29,7 @@
 
 #include "mlir/IR/Dialect.h"
 #include "mlir/Pass/PassManager.h"
+#include "mlir/Target/LLVMIR/ModuleTranslation.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
 #include 
@@ -407,6 +409,72 @@
   ci.semantics().DumpSymbolsSources(llvm::outs());
 }
 
+#include "flang/Tools/CLOptions.inc"
+
+// Lower the previously generated MLIR module into an LLVM IR module
+void CodeGenAction::GenerateLLVMIR() {
+  assert(mlirModule && "The MLIR module has not been generated yet.");
+
+  CompilerInstance &ci = this->instance();
+
+  fir::support::loadDialects(*mlirCtx);
+  fir::support::registerLL

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682
+Expr::EvalResult EVRX, EVRY;
+if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) ||
+!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C))
+  return false;
+
+APValue VX = EVRX.Val, VY = EVRY.Val;
+if (VX.getKind() != VY.getKind())

urnathan wrote:
> ChuanqiXu wrote:
> > urnathan wrote:
> > > I'm kind of surprised how complex this check is.  Isn't there an AST 
> > > comparator available somewhere?
> > I found ODRHash. I think it is much better now.
> hm that suggests there there must be a comparator too -- this isn't a 
> cryptographically strong hash is it?  How would the compiler currently make 
> use of 'definitely different' and 'probably the same' without such a 
> comparator?
Yeah, I am sure there is not an such comparator. Or it has some methods like: 
`ASTContext::hasSameType` for type and `ASTContext::isSameEntity()` for Decl. 
But it lacks such methods now for Stmt and Expr.

> How would the compiler currently make use of 'definitely different' and 
> 'probably the same' without such a comparator?

Now it uses the two methods I listed above and ODRHash to compare. I think the 
two methods works for  'definitely different' and ODRHash works for 'probably 
the same'. So it's the reason why my previous implementation looks lengthy. 
Since I want to handle it by hand. (The previous method only works for simple 
Expr. I think it would be large work to implement comparator for whole Expr or 
Stmt).


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

https://reviews.llvm.org/D118034

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

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:177
+  if (IsPartition) {
+ModuleName += ":";
+ModuleName += stringFromPath(Partition);

iains wrote:
> ChuanqiXu wrote:
> > I chose '-' in my implementation since here ModuleName refers to the name 
> > in filesystem, right? And I remember '-' is the choice of GCC. So let's 
> > keep consistency.
> This is not my understanding.
> 
> ModuleName is the "user-facing" name of the module that is described in the 
> source, and we use this in diagnostics etc.
> 
> The translation of that name to an on-disk name can be arbitrary and there 
> are several mechanisms in clang already (e.g. -fmodule-file=A:B=foo.pcm) the 
> module loader uses these to find the module on the basis of its user-facing 
> name where required.
> 
> When we have P1184, it is even more important that the interface is supplied 
> with the name that the user will put into the source code.
> 
> 
I agree with Iain, we should use ':' is module names here.  When mapping a 
named module to the file system some translation is needed because ':' is not 
permitted in file names on some filesystems (you know the ones!)



Comment at: clang/lib/Sema/SemaModule.cpp:350
+  bool IsPartition = !Partition.empty();
+  bool Cxx20Mode = getLangOpts().CPlusPlusModules || getLangOpts().ModulesTS;
+  assert((!IsPartition || Cxx20Mode) && "partition seen in non-C++20 code?");

iains wrote:
> ChuanqiXu wrote:
> > I prefer to remove the support for getLangOpts().ModulesTS on the fly.
> agreed in comments, but I think it is outside of my remit to remove the 
> functionality?
We can't just drop the -fmodules-ts option.  I think its semantics should 
be[come] the same as CPlusPlusModules.  That's not a change for this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118586

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

[PATCH] D120032: [OpenCL] opencl-c.h: use uint/ulong consistently

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120032

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

[PATCH] D118977: [NVPTX] Add more FMA intriniscs/builtins

[Jakub Chlanda via Phabricator via cfe-commits]

jchlanda added a comment.

@tra I've fixed the test failure (`math-intrins.ll`) the rest seems to be 
unrelated timeouts, would you be able to merge those patches in, as I don't 
have the commit access please? The same goes for 
https://reviews.llvm.org/D117887 and https://reviews.llvm.org/D119157 from 
Nicolas. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118977

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

[PATCH] D119918: [CMake] Rename TARGET_TRIPLE to LLVM_TARGET_TRIPLE

[Diana Picus via Phabricator via cfe-commits]

rovka accepted this revision.
rovka added a comment.

I'm happy if the buildbots are happy :D thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119918

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

[PATCH] D111100: enable plugins for clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/test/CMakeLists.txt:91
+  if(TARGET CTTestTidyModule)
+  list(APPEND CLANG_TOOLS_TEST_DEPS CTTestTidyModule LLVMHello)
+  target_include_directories(CTTestTidyModule PUBLIC BEFORE 
"${CLANG_TOOLS_SOURCE_DIR}")

tstellar wrote:
> Our stand-alone builds for Fedora are still not working after this patch, 
> even with D119199.  Why is it necesary to add LLVMHello as a test dependency? 
>  Which test is using it?
CTestTidyModule.cpp is using it:
```
// RUN: clang-tidy -checks='-*,mytest*' --list-checks -load 
%llvmshlibdir/CTTestTidyModule%pluginext -load 
%llvmshlibdir/LLVMHello%pluginext | FileCheck --check-prefix=CHECK-LIST %s
```
I *think* this is testing that we can load a clang-tidy plugin and an llvm 
plugin at the same time and not hit conflicting symbols or other issues. If I'm 
correct, then I think that's useful test functionality, but I wouldn't describe 
it as critical, so I'd be fine if we dropped it for now to get this patch in, 
and then added the extra testing in a subsequent patch if we think it's 
necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[clang] 2614de8 - [clang] CGCXXABI::EmitLoadOfMemberFunctionPointer - use castAs<> instead of getAs<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-17T13:18:23Z
New Revision: 2614de82025bd9c04f8515747a611238c0ac4e05

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

LOG: [clang] CGCXXABI::EmitLoadOfMemberFunctionPointer - use castAs<> instead 
of getAs<> to avoid dereference of nullptr

The pointer is always dereferenced by arrangeCXXMethodType, so assert the cast 
is correct instead of returning nullptr

Added: 


Modified: 
clang/lib/CodeGen/CGCXXABI.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCXXABI.cpp b/clang/lib/CodeGen/CGCXXABI.cpp
index 0b441e382f11..42e6c916bed0 100644
--- a/clang/lib/CodeGen/CGCXXABI.cpp
+++ b/clang/lib/CodeGen/CGCXXABI.cpp
@@ -45,8 +45,7 @@ CGCallee CGCXXABI::EmitLoadOfMemberFunctionPointer(
   ErrorUnsupportedABI(CGF, "calls through member pointers");
 
   ThisPtrForCall = This.getPointer();
-  const FunctionProtoType *FPT =
-MPT->getPointeeType()->getAs();
+  const auto *FPT = MPT->getPointeeType()->castAs();
   const auto *RD =
   cast(MPT->getClass()->castAs()->getDecl());
   llvm::FunctionType *FTy = CGM.getTypes().GetFunctionType(



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

[clang] 57fc979 - [clang] CGDebugInfo::getOrCreateMethodType - use castAs<> instead of getAs<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-17T13:18:23Z
New Revision: 57fc9798d7145626809b0e81af9154a755b383eb

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

LOG: [clang] CGDebugInfo::getOrCreateMethodType - use castAs<> instead of 
getAs<> to avoid dereference of nullptr

The pointer is always dereferenced, so assert the cast is correct instead of 
returning nullptr

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index c09adad09aa8..d75b5a1a9d12 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1725,7 +1725,7 @@ void CGDebugInfo::CollectRecordFields(
 llvm::DISubroutineType *
 CGDebugInfo::getOrCreateMethodType(const CXXMethodDecl *Method,
llvm::DIFile *Unit, bool decl) {
-  const FunctionProtoType *Func = 
Method->getType()->getAs();
+  const auto *Func = Method->getType()->castAs();
   if (Method->isStatic())
 return cast_or_null(
 getOrCreateType(QualType(Func, 0), Unit));



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

[PATCH] D119926: [Clang][AArch64] Enable _Float16 _Complex type

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/CodeGen/aarch64-complex-half-math.c:3
+// RUN: %clang_cc1 %s -emit-llvm -triple aarch64-unknown-unknown -o - | 
FileCheck %s --check-prefix=AARCH64
+// REQUIRES: aarch64-registered-target
+

There's nothing AArch64-specific about the change, so I'd expect some more 
general codegen tests.

Actually, I took a look to see what calls `getFloatingTypeOfSizeWithinDomain()` 
and I can't find any callers of that. Do you have a test case where you were 
hitting that particular unreachable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119926

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

[PATCH] D114425: [clang] Add __builtin_bswap128

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D114425#3225908 , @erichkeane 
wrote:

> In D114425#3225892 , @craig.topper 
> wrote:
>
>> I kind of wonder if we should detect the __int128 type being requested in 
>> ASTContext::GetBuiltinType and return an error up to 
>> Sema::LazilyCreateBuiltin. Probably requires a new error code and handling 
>> for it in LazilyCreateBuiltin. I assume that would catch the bad builtin 
>> earlier. As it stands now we'd still allow it if it constant folds away in 
>> ExprConstant.cpp so that it never reaches CGBuiltin.cpp. But I'm not a 
>> frontend expert. Adding more potential reviewers
>
> I think I like that idea, "DecodeTypeFromStr" should probably test if 
> __int128 is supported.  Having us only diagnose in codegen is the wrong 
> approach here, we need to reject it in Sema.
>
> I would also want to see some IR-tests to show how this looks in IR, 
> particularly one where it takes an __int128 as parameter (plus others that 
> show the return value is valid?).

Where should I put the tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

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

[PATCH] D114425: [clang] Add __builtin_bswap128

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 409607.
philnik added a comment.

- Rebased
- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins.cpp
  clang/test/Sema/constant-builtins-2.c

Index: clang/test/Sema/constant-builtins-2.c
===
--- clang/test/Sema/constant-builtins-2.c
+++ clang/test/Sema/constant-builtins-2.c
@@ -216,6 +216,9 @@
 int h3 = __builtin_bswap16(0x1234) == 0x3412 ? 1 : f();
 int h4 = __builtin_bswap32(0x1234) == 0x3412 ? 1 : f();
 int h5 = __builtin_bswap64(0x1234) == 0x3412 ? 1 : f();
+#if defined(__SIZEOF_INT128__)
+int h6 = __builtin_bswap128(0x1234) == (((__int128)0x3412) << 112) ? 1 : f();
+#endif
 extern long int bi0;
 extern __typeof__(__builtin_expect(0, 0)) bi0;
 
Index: clang/test/CodeGen/builtins.cpp
===
--- clang/test/CodeGen/builtins.cpp
+++ clang/test/CodeGen/builtins.cpp
@@ -20,6 +20,10 @@
 decltype(__builtin_bswap32(0)) bswap32 = 42;
 extern uint64_t bswap64;
 decltype(__builtin_bswap64(0)) bswap64 = 42;
+#ifdef __SIZEOF_INT128__
+extern __uint128_t bswap128;
+decltype(__builtin_bswap128(0)) bswap128 = 42;
+#endif
 
 #ifdef __clang__
 extern uint8_t bitrev8;
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2929,7 +2929,8 @@
   }
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
-  case Builtin::BI__builtin_bswap64: {
+  case Builtin::BI__builtin_bswap64:
+  case Builtin::BI__builtin_bswap128: {
 return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::bswap));
   }
   case Builtin::BI__builtin_bitreverse8:
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11784,7 +11784,8 @@
 
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
-  case Builtin::BI__builtin_bswap64: {
+  case Builtin::BI__builtin_bswap64:
+  case Builtin::BI__builtin_bswap128: {
 APSInt Val;
 if (!EvaluateInteger(E->getArg(0), Val, Info))
   return false;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -10840,7 +10840,8 @@
 static QualType DecodeTypeFromStr(const char *&Str, const ASTContext &Context,
   ASTContext::GetBuiltinTypeError &Error,
   bool &RequiresICE,
-  bool AllowTypeModifiers) {
+  bool AllowTypeModifiers,
+  bool AllowInt128 = false) {
   // Modifiers.
   int HowLong = 0;
   bool Signed = false, Unsigned = false;
@@ -10983,9 +10984,13 @@
   Type = Context.ShortTy;
 break;
   case 'i':
-if (HowLong == 3)
+if (HowLong == 3) {
+  if (!AllowInt128 && !Context.getTargetInfo().hasInt128Type()) {
+Error = ASTContext::GE_Missing_type;
+return {};
+  }
   Type = Unsigned ? Context.UnsignedInt128Ty : Context.Int128Ty;
-else if (HowLong == 2)
+} else if (HowLong == 2)
   Type = Unsigned ? Context.UnsignedLongLongTy : Context.LongLongTy;
 else if (HowLong == 1)
   Type = Unsigned ? Context.UnsignedLongTy : Context.LongTy;
@@ -11065,7 +11070,7 @@
 Str = End;
 
 QualType ElementType = DecodeTypeFromStr(Str, Context, Error,
- RequiresICE, false);
+ RequiresICE, false, true);
 assert(!RequiresICE && "Can't require vector ICE");
 
 // TODO: No way to make AltiVec vectors in builtins yet.
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -513,6 +513,7 @@
 BUILTIN(__builtin_bswap16, "UsUs", "nc")
 BUILTIN(__builtin_bswap32, "UZiUZi", "nc")
 BUILTIN(__builtin_bswap64, "UWiUWi", "nc")
+BUILTIN(__builtin_bswap128, "ULLLiULLLi", "nc")
 
 BUILTIN(__builtin_bitreverse8, "UcUc", "nc")
 BUILTIN(__builtin_bitreverse16, "UsUs", "nc")
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -60,6 +60,14 @@
 Non-comprehensive list of changes in this release
 -
 
+- Maximum _ExtInt size was decreased f

[clang-tools-extra] 1c502c6 - [clang-doc] SerializeIndex - pass Index param by constant reference

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-17T13:28:02Z
New Revision: 1c502c63cb77dd15e698087fdc6b3fb892ce0977

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

LOG: [clang-doc] SerializeIndex - pass Index param by constant reference

Silence coverity warnings about unnecessary copies

Added: 


Modified: 
clang-tools-extra/clang-doc/HTMLGenerator.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-doc/HTMLGenerator.cpp 
b/clang-tools-extra/clang-doc/HTMLGenerator.cpp
index e110f312d10c4..4ab962be7864d 100644
--- a/clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ b/clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -899,7 +899,7 @@ static llvm::Error SerializeIndex(ClangDocContext &CDCtx) {
   }
   CDCtx.Idx.sort();
   llvm::json::OStream J(OS, 2);
-  std::function IndexToJSON = [&](Index I) {
+  std::function IndexToJSON = [&](const Index &I) {
 J.object([&] {
   J.attribute("USR", toHex(llvm::toStringRef(I.USR)));
   J.attribute("Name", I.Name);



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

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

[Alex Bradbury via Phabricator via cfe-commits]

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

In that case, LGTM (needs a rebase though). Thanks for your patience on this 
@achieveartificialintelligence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

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

[PATCH] D114425: [clang] Add __builtin_bswap128

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

xbolva00 added a comment.

In D114425#3329303 , @philnik wrote:

> In D114425#3225908 , @erichkeane 
> wrote:
>
>> In D114425#3225892 , @craig.topper 
>> wrote:
>>
>>> I kind of wonder if we should detect the __int128 type being requested in 
>>> ASTContext::GetBuiltinType and return an error up to 
>>> Sema::LazilyCreateBuiltin. Probably requires a new error code and handling 
>>> for it in LazilyCreateBuiltin. I assume that would catch the bad builtin 
>>> earlier. As it stands now we'd still allow it if it constant folds away in 
>>> ExprConstant.cpp so that it never reaches CGBuiltin.cpp. But I'm not a 
>>> frontend expert. Adding more potential reviewers
>>
>> I think I like that idea, "DecodeTypeFromStr" should probably test if 
>> __int128 is supported.  Having us only diagnose in codegen is the wrong 
>> approach here, we need to reject it in Sema.
>>
>> I would also want to see some IR-tests to show how this looks in IR, 
>> particularly one where it takes an __int128 as parameter (plus others that 
>> show the return value is valid?).
>
> Where should I put the tests?

test/CodeGen

Test with -emit-llvm. There are other similar tests :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

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

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 409614.
iains marked 9 inline comments as done.
iains added a comment.

address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118586

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticIDs.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CXX/module/module.unit/p3.cpp
  clang/test/CXX/module/module.unit/p8.cpp
  clang/test/Modules/cxx20-partition-diagnostics-a.cpp

Index: clang/test/Modules/cxx20-partition-diagnostics-a.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-partition-diagnostics-a.cpp
@@ -0,0 +1,18 @@
+// Module Partition diagnostics
+
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -D TU=1 -x c++ %s -verify
+
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -D TU=2 -x c++ %s -verify
+
+#if TU == 1
+
+import :B; // expected-error {{module partition imports must be within a module purview}}
+
+#elif TU == 2
+module; // expected-error {{missing 'module' declaration at end of global module fragment introduced here}}
+
+import :Part; // expected-error {{module partition imports cannot be in the global module fragment}}
+
+#else
+#error "no TU set"
+#endif
Index: clang/test/CXX/module/module.unit/p8.cpp
===
--- clang/test/CXX/module/module.unit/p8.cpp
+++ clang/test/CXX/module/module.unit/p8.cpp
@@ -12,7 +12,7 @@
 
 #elif MODE == 1
 // expected-no-diagnostics
-module foo;
+module foo; // Implementation, implicitly imports foo.
 #define IMPORTED
 
 #elif MODE == 2
@@ -21,15 +21,15 @@
 #define IMPORTED
 
 #elif MODE == 3
-export module bar;
+export module bar; // A different module
 
 #elif MODE == 4
-module foo:bar; // expected-error {{not yet supported}}
-#define IMPORTED // FIXME
+module foo:bar; // Partition implementation
+//#define IMPORTED (we don't import foo here)
 
 #elif MODE == 5
-export module foo:bar; // expected-error {{not yet supported}} expected-error {{redefinition}} expected-note@* {{loaded from}}
-#define IMPORTED // FIXME
+export module foo:bar; // Partition interface
+//#define IMPORTED  (we don't import foo here)
 
 #endif
 
Index: clang/test/CXX/module/module.unit/p3.cpp
===
--- clang/test/CXX/module/module.unit/p3.cpp
+++ clang/test/CXX/module/module.unit/p3.cpp
@@ -1,4 +1,4 @@
 // RUN: %clang_cc1 -std=c++2a -verify %s
 
-export module foo:bar; // expected-error {{sorry, module partitions are not yet supported}}
-import :baz; // expected-error {{sorry, module partitions are not yet supported}}
+export module foo:bar;
+import :baz; // expected-error {{module 'foo:baz' not found}}
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -54,6 +54,23 @@
   }
 }
 
+// We represent the primary and partition names as 'Paths' which are sections
+// of the hierarchical access path for a clang module.  However for C++20
+// the periods in a name are just another character, and we will need to
+// flatten them into a string.
+static std::string stringFromPath(ModuleIdPath Path) {
+  std::string Name;
+  if (Path.empty())
+return Name;
+
+  for (auto &Piece : Path) {
+if (!Name.empty())
+  Name += ".";
+Name += Piece.first->getName();
+  }
+  return Name;
+}
+
 Sema::DeclGroupPtrTy
 Sema::ActOnGlobalModuleFragmentDecl(SourceLocation ModuleLoc) {
   if (!ModuleScopes.empty() &&
@@ -80,11 +97,10 @@
   return nullptr;
 }
 
-Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation StartLoc,
-   SourceLocation ModuleLoc,
-   ModuleDeclKind MDK,
-   ModuleIdPath Path,
-   ModuleImportState &ImportState) {
+Sema::DeclGroupPtrTy
+Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
+  ModuleDeclKind MDK, ModuleIdPath Path,
+  ModuleIdPath Partition, ModuleImportState &ImportState) {
   assert((getLangOpts().ModulesTS || getLangOpts().CPlusPlusModules) &&
  "should only have module decl in Modules TS or C++20");
 
@@ -163,19 +179,20 @@
   // Flatten the dots in a module name. Unlike Clang's hierarchical module map
   // modules, the dots here are just another character that can appear in a
   // module name.
-  std::string ModuleName;
-  for (auto &Piece : Path) {
-if (!ModuleName.empty())
-  ModuleName += ".";
-ModuleName += Piece.first->getName();
+  std::string ModuleName = stringFromPath(Path);
+  bool IsP

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

[Iain Sandoe via Phabricator via cfe-commits]

iains added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:177
+  if (IsPartition) {
+ModuleName += ":";
+ModuleName += stringFromPath(Partition);

urnathan wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > I chose '-' in my implementation since here ModuleName refers to the name 
> > > in filesystem, right? And I remember '-' is the choice of GCC. So let's 
> > > keep consistency.
> > This is not my understanding.
> > 
> > ModuleName is the "user-facing" name of the module that is described in the 
> > source, and we use this in diagnostics etc.
> > 
> > The translation of that name to an on-disk name can be arbitrary and there 
> > are several mechanisms in clang already (e.g. -fmodule-file=A:B=foo.pcm) 
> > the module loader uses these to find the module on the basis of its 
> > user-facing name where required.
> > 
> > When we have P1184, it is even more important that the interface is 
> > supplied with the name that the user will put into the source code.
> > 
> > 
> I agree with Iain, we should use ':' is module names here.  When mapping a 
> named module to the file system some translation is needed because ':' is not 
> permitted in file names on some filesystems (you know the ones!)
(just to expand a little more)

the on-disk name needs to be chosen suitably for the platform and by the user 
and/or the build system.

When the FE chooses a default filename (which is done in building jobs, not in 
the Parser of course) it chooses one based on the source filename.  It would be 
most unfortunate if the Parser/Sema needed to understand platform filename 
rules.

When you do  'clang -module-file-info' (with the existing or updated version) 
you should see the module name as per the source code, the translation would 
only apply to the filename itself.

- to answer a later comment:

when we do -fmodule-file=A_B.pcm  and A_B.pcm contains a module named A:B the 
loader notices this pairing when it pre-loads the module, so that when we ask 
for "A:B" the loader already knows which on-disk file contains. it.

if we use the HeaderSearch mechanisms (when we want to figure out a 
module-name<=> filename pairing without loading the module) we can use 
-fmodule-name=A:B=A_B.pcm,

These mechanisms work today, but P1184 is a more flexible mechanism and avoids 
having  massive command lines with many -fmodule-file= cases.




Comment at: clang/lib/Sema/SemaModule.cpp:350
+  bool IsPartition = !Partition.empty();
+  bool Cxx20Mode = getLangOpts().CPlusPlusModules || getLangOpts().ModulesTS;
+  assert((!IsPartition || Cxx20Mode) && "partition seen in non-C++20 code?");

urnathan wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > I prefer to remove the support for getLangOpts().ModulesTS on the fly.
> > agreed in comments, but I think it is outside of my remit to remove the 
> > functionality?
> We can't just drop the -fmodules-ts option.  I think its semantics should 
> be[come] the same as CPlusPlusModules.  That's not a change for this patch.
that was what I expected.



Comment at: clang/lib/Sema/SemaModule.cpp:359
+
   std::string ModuleName;
+  if (IsPartition) {

ChuanqiXu wrote:
> I prefer to move the variable to the following block.
again the code is reorganised in 4/8



Comment at: clang/lib/Sema/SemaModule.cpp:433-451
+  if (NamePath.empty()) {
+// If this was a header import, pad out with dummy locations.
+// FIXME: Pass in and use the location of the header-name token in this
+// case.
+for (Module *ModCheck = Mod; ModCheck; ModCheck = ModCheck->Parent)
   IdentifierLocs.push_back(SourceLocation());
+  } else if (getLangOpts().CPlusPlusModules && !Mod->Parent) {

ChuanqiXu wrote:
> I don't find the reason to refactor here. It looks NFC and I think the 
> original form is simpler.
it is not NFC:
1. we add the C++20 case (the name is a single identifier)
2. we avoid repeating the check for an empty Path (the first loop would 
effectively check that too).




Comment at: clang/test/Modules/cxx20-import-diagnostics-a.cpp:57
 
+module;
+

ChuanqiXu wrote:
> What if we don't add this? I think the original is good. So we should add a 
> new test if we feel needed.
OK, probably it should have been in from the start - but we will also check 
these things in the tests taken from the standard, so I don't think it is 
important here; removed.



Comment at: clang/test/Modules/cxx20-partition-diagnostics-a.cpp:3-5
+// RUN: %clang_cc1 -std=c++20 -S -D TU=1 -x c++ %s -o /dev/null -verify
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=2 -x c++ %s -o /dev/null -verify

ChuanqiXu wrote:
> We could use `-fsyntax-only` instead of `-o /dev/null`
heh, I usually do this not sure why I did not here; changed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118

[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

In D91605#3322312 , @ro wrote:

> In D91605#3321554 , @MaskRay wrote:
>
>> `GetTls` is about the static TLS block size. It consists of the main 
>> executable's TLS, and initially loaded shared objects' TLS, `struct 
>> pthread`, and padding.
>> Solaris should be able to just use the code path like FreeBSD, Linux musl, 
>> and various unpopular architectures of Linux glibc:
>>
>>   #elif SANITIZER_FREEBSD || SANITIZER_LINUX
>> uptr align;
>> GetStaticTlsBoundary(addr, size, &align);
>
> Not unconditionally, unfortunally: as the comment above `GetSizeFromHdr` 
> explains, `dlpi_tls_modid` was only introduced in an update to Solaris 11.4 
> FCS (which is sort of a problem), but isn't present in 11.3 (don't reallly 
> care) and Illumos (this would break compilation for them).  OTOH my solution 
> is successfully being used in GCC's `libphobos` on Solaris 11.3, 11.4, and 
> most likely Illumos, too.  I'd rather not burn the Illumos bridge if it can 
> be avoided.

As an experiment, I've tried to use `GetStaticTlsBoundary` instead.  It does 
indeed work on recent Solaris 11.4 and the resulting patch is at D120048 
.  I'm pretty certain that support for 
Solaris 11.3/Illumos which lack `dlpi_tls_modid` using `dlinfo(RTLD_SELF, 
RTLD_DI_LINKMAP)` can be added on top of that one, unbreaking the Illumos 
build.  This would avoid considerable duplication of the `dl_iterate_phdr` 
code, which is certainly a nice benefit.  I'll experiment with that route later.

I know now why I did the present patch the way it is: `GetStaticTlsBoundary` 
was only introduced months after I submitted this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91605

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

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

[Shao-Ce SUN via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
achieveartificialintelligence marked an inline comment as done.
Closed by commit rG7798ecca9c3d: [RISCV] add the MC layer support of Zfinx 
extension (authored by achieveartificialintelligence).

Changed prior to commit:
  https://reviews.llvm.org/D93298?vs=407482&id=409617#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

Files:
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoD.td
  llvm/lib/Target/RISCV/RISCVInstrInfoF.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZfh.td
  llvm/lib/Target/RISCV/RISCVRegisterInfo.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32i-invalid.s
  llvm/test/MC/RISCV/rv32zdinx-invalid.s
  llvm/test/MC/RISCV/rv32zdinx-valid.s
  llvm/test/MC/RISCV/rv32zfinx-invalid.s
  llvm/test/MC/RISCV/rv32zfinx-valid.s
  llvm/test/MC/RISCV/rv32zhinx-invalid.s
  llvm/test/MC/RISCV/rv32zhinx-valid.s
  llvm/test/MC/RISCV/rv32zhinxmin-invalid.s
  llvm/test/MC/RISCV/rv32zhinxmin-valid.s
  llvm/test/MC/RISCV/rv64zdinx-invalid.s
  llvm/test/MC/RISCV/rv64zdinx-valid.s
  llvm/test/MC/RISCV/rv64zfinx-invalid.s
  llvm/test/MC/RISCV/rv64zfinx-valid.s
  llvm/test/MC/RISCV/rv64zhinx-invalid.s
  llvm/test/MC/RISCV/rv64zhinx-valid.s
  llvm/test/MC/RISCV/rv64zhinxmin-invalid.s
  llvm/test/MC/RISCV/rv64zhinxmin-valid.s
  llvm/test/MC/RISCV/rvzdinx-aliases-valid.s
  llvm/test/MC/RISCV/rvzfinx-aliases-valid.s
  llvm/test/MC/RISCV/rvzhinx-aliases-valid.s

Index: llvm/test/MC/RISCV/rvzhinx-aliases-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvzhinx-aliases-valid.s
@@ -0,0 +1,82 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+zhinx -riscv-no-aliases \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+zhinx \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+zhinx -riscv-no-aliases \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+zhinx \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+zhinx -M no-aliases - \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+zhinx - \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+zhinx -M no-aliases - \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+zhinx - \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+
+##===--===##
+## Assembler Pseudo Instructions (User-Level ISA, Version 2.2, Chapter 20)
+##===--===##
+
+# CHECK-INST: fsgnjx.h s1, s2, s2
+# CHECK-ALIAS: fabs.h s1, s2
+fabs.h s1, s2
+# CHECK-INST: fsgnjn.h s2, s3, s3
+# CHECK-ALIAS: fneg.h s2, s3
+fneg.h s2, s3
+
+# CHECK-INST: flt.h tp, s6, s5
+# CHECK-ALIAS: flt.h tp, s6, s5
+fgt.h x4, s5, s6
+# CHECK-INST: fle.h t2, s1, s0
+# CHECK-ALIAS: fle.h t2, s1, s0
+fge.h x7, x8, x9
+
+##===--===##
+## Aliases which omit the rounding mode.
+##===--===##
+
+# CHECK-INST: fmadd.h a0, a1, a2, a3, dyn
+# CHECK-ALIAS: fmadd.h a0, a1, a2, a3
+fmadd.h x10, x11, x12, x13
+# CHECK-INST: fmsub.h a4, a5, a6, a7, dyn
+# CHECK-ALIAS: fmsub.h a4, a5, a6, a7
+fmsub.h x14, x15, x16, x17
+# CHECK-INST: fnmsub.h s2, s3, s4, s5, dyn
+# CHECK-ALIAS: fnmsub.h s2, s3, s4, s5
+fnmsub.h x18, x19, x20, x21
+# CHECK-INST: fnmadd.h s6, s7, s8, s9, dyn
+# CHECK-ALIAS: fnmadd.h s6, s7, s8, s9
+fnmadd.h x22, x23, x24, x25
+# CHECK-INST: fadd.h s10, s11, t3, dyn
+# CHECK-ALIAS: fadd.h s10, s11, t3
+fadd.h x26, x27, x28
+# CHECK-INST: fsub.h t4, t5, t6, dyn
+# CHECK-ALIAS: fsub.h t4, t5, t6
+fsub.h x29, x30, x31
+# CHECK-INST: fmul.h s0, s1, s2, dyn
+# CHECK-ALIAS: fmul.h s0, s1, s2
+fmul.h s0, s1, s2
+# CHECK-INST: fdiv.h s3, s4, s5, dyn
+# CHECK-ALIAS: fdiv.h s3, s4, s5
+fdiv.h s3, s4, s5
+# CHECK-INST: fsqrt.h s6, s7, dyn
+# CHECK-ALIAS: fsqrt.h s6, s7
+fsqrt.h s6, s7
+# CHECK-INST: fcvt.w.h a0, s5, dyn
+# CHECK-ALIAS: fcvt.w.h a0, s5
+fcvt.w.h a0, s5
+# CHECK-INST: fcvt.wu.h a1, s6, dyn
+# CHECK-ALIAS: fcvt.wu.h a1, s6
+fcv

[PATCH] D119989: [clangd] Fix building SerializationTests unit test on OpenBSD

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks




Comment at: clang-tools-extra/clangd/unittests/SerializationTests.cpp:313
 // Sanitizers use a lot of address space, so we can't apply strict limits.
-#if LLVM_ON_UNIX && !LLVM_ADDRESS_SANITIZER_BUILD &&   
\
+#if LLVM_ON_UNIX && defined(RLIMIT_AS) && !LLVM_ADDRESS_SANITIZER_BUILD && 
\
 !LLVM_MEMORY_SANITIZER_BUILD

can you also add a comment saying `RLIMIT_AS` doesn't exist in OpenBSD ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119989

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

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

[Nathan Sidwell via Phabricator via cfe-commits]

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

thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118586

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

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

[Shao-Ce SUN via Phabricator via cfe-commits]

achieveartificialintelligence added a comment.

Thanks all!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

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

[PATCH] D118333: [RISCV] Use computeTargetABI from llc as well as clang

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

Thanks, I've put this on the agenda for the RISC-V LLVM sync call today. I 
think this is more attractive than the previous proposal due to unifying logic 
between llc and Clang. I could see a counter-argument about llc being a 
low-level tool that should be controlled very explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118333

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

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682
+Expr::EvalResult EVRX, EVRY;
+if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) ||
+!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C))
+  return false;
+
+APValue VX = EVRX.Val, VY = EVRY.Val;
+if (VX.getKind() != VY.getKind())

ChuanqiXu wrote:
> urnathan wrote:
> > ChuanqiXu wrote:
> > > urnathan wrote:
> > > > I'm kind of surprised how complex this check is.  Isn't there an AST 
> > > > comparator available somewhere?
> > > I found ODRHash. I think it is much better now.
> > hm that suggests there there must be a comparator too -- this isn't a 
> > cryptographically strong hash is it?  How would the compiler currently make 
> > use of 'definitely different' and 'probably the same' without such a 
> > comparator?
> Yeah, I am sure there is not an such comparator. Or it has some methods like: 
> `ASTContext::hasSameType` for type and `ASTContext::isSameEntity()` for Decl. 
> But it lacks such methods now for Stmt and Expr.
> 
> > How would the compiler currently make use of 'definitely different' and 
> > 'probably the same' without such a comparator?
> 
> Now it uses the two methods I listed above and ODRHash to compare. I think 
> the two methods works for  'definitely different' and ODRHash works for 
> 'probably the same'. So it's the reason why my previous implementation looks 
> lengthy. Since I want to handle it by hand. (The previous method only works 
> for simple Expr. I think it would be large work to implement comparator for 
> whole Expr or Stmt).
Hm, how do template instantations work -- there must be some way of determining 
'is this instantation just here the same as one I've already seen'


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

https://reviews.llvm.org/D118034

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

[PATCH] D114425: [clang] Add __builtin_bswap128

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:10987
   case 'i':
-if (HowLong == 3)
+if (HowLong == 3) {
+  if (!AllowInt128 && !Context.getTargetInfo().hasInt128Type()) {

Please add a test in Sema as well to validate the diagnostic so we can ensure 
it sounds sane.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

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

[PATCH] D119077: clangd SemanticHighlighting: added support for highlighting overloaded operators

[Iannis de Zwart via Phabricator via cfe-commits]

iannisdezwart requested review of this revision.
iannisdezwart added a comment.

Would be nice if this can further be reviewed and commited to the repo!


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

https://reviews.llvm.org/D119077

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

[PATCH] D119525: [clang] Fix crash when array size is missing in initializer

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:54
+--
+- ``CXXNewExpr::getArraySize()`` previously returned a ``llvm::Optional``
+  wrapping a ``nullptr`` when the ``CXXNewExpr`` did not have an array

tbaeder wrote:
> aaron.ballman wrote:
> > Just added a reference to the bug that was fixed.
> I was wondering about this. Where does the "PRXX" syntax come from? Since 
> the repo and issues are on github now, I don't see how it makes sense to call 
> it a PR (it's an issue, not a pull request) and github doesn't linkify those, 
> while it does when using the `#xx` syntax (which isn't relevant in this 
> case, but it is when using it in git commit messages). I have seen other 
> people use the same syntax to refer to issues. I'd probably just add an 
> actual link to the github issue here if that's permitted. Thanks for the 
> suggestion though, I'm on PTO this week so don't land this until Monday  :)
> I was wondering about this. Where does the "PRXX" syntax come from?

"Problem Report" -- ancient terminology.

> I have seen other people use the same syntax to refer to issues. I'd probably 
> just add an actual link to the github issue here if that's permitted.

TBH, I think that's an even better suggestion (linking to the issue). One 
concern I have is that it's really hard to tell whether the number is a 
bugzilla issue number or a GitHub issue number (I suppose we should all assume 
they're always github issue numbers these days though), so I wasn't keen on 
having a number with no prefix to it. But if we're linking to what's been 
fixed, then there's never a chance for confusion.

> Thanks for the suggestion though, I'm on PTO this week so don't land this 
> until Monday :)

Sounds good to me, enjoy your PTO!


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

https://reviews.llvm.org/D119525

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

[PATCH] D119612: [clang] Pass more flags to ld64.lld

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

lld-macho folks, any concerns with this?


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

https://reviews.llvm.org/D119612

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

[PATCH] D118587: [C++20][Modules][4/8] Handle generation of partition implementation CMIs.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 409627.
iains marked 2 inline comments as done.
iains added a comment.

address review comments, rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118587

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaModule.cpp
  clang/test/Modules/cxx20-10-1-ex1.cpp
  clang/test/Modules/cxx20-10-1-ex2.cpp
  clang/test/Modules/cxx20-import-diagnostics-a.cpp

Index: clang/test/Modules/cxx20-import-diagnostics-a.cpp
===
--- clang/test/Modules/cxx20-import-diagnostics-a.cpp
+++ clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -94,9 +94,9 @@
 #elif TU == 6
 
 module;
-// We can only have preprocessor commands here, which could include an include
+// We can only have preprocessor directives here, which permits an include-
 // translated header unit.  However those are identified specifically by the
-// preprocessor; non-preprocessed user code should not contain an import here.
+// preprocessor; non-preprocessed user code should not contain an 'import' here.
 import B; // expected-error {{module imports cannot be in the global module fragment}}
 
 export module D;
Index: clang/test/Modules/cxx20-10-1-ex2.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-10-1-ex2.cpp
@@ -0,0 +1,60 @@
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN:  -o %t/B_Y.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN:  -fmodule-file=%t/B_Y.pcm -o %t/B.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=3 -x c++ %s \
+// RUN:   -o %t/B_X1.pcm -verify
+
+// Not expected to work yet.
+//  %clang_cc1 -std=c++20 -emit-module-interface -D TU=4 -x c++ %s \
+//   -fmodule-file=%t/B.pcm  -o %t/B_X2.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -D TU=5 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm  -o %t/b_tu5.s
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=6 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm  -o %t/b_tu6.s -verify
+
+// Not expected to work yet.
+//  %clang_cc1 -std=c++20 -emit-module-interface -D TU=7 -x c++ %s \
+//   -fmodule-file=%t/B_X2.pcm  -o %t/B_X3.pcm -verify
+
+#if TU == 1
+  module B:Y;
+  int y();
+// expected-no-diagnostics
+#elif TU == 2
+  export module B;
+  import :Y;
+  int n = y();
+// expected-no-diagnostics
+#elif TU == 3
+  module B:X1; // does not implicitly import B
+  int &a = n;  // expected-error {{use of undeclared identifier }}
+#elif TU == 4
+  module B:X2; // does not implicitly import B
+  import B;
+  int &b = n; // OK
+// expected-no-diagnostics
+#elif TU == 5
+  module B; // implicitly imports B
+  int &c = n; // OK
+// expected-no-diagnostics
+#elif TU == 6
+  import B;
+  // error, n is module-local and this is not a module.
+  int &c = n; // expected-error {{use of undeclared identifier}}
+#elif TU == 7
+  module B:X3; // does not implicitly import B
+  import :X2; // X2 is an implementation so exports nothing.
+  // error: n not visible here.
+  int &c = n; // expected-error {{use of undeclared identifier }}
+#else
+#error "no TU set"
+#endif
Index: clang/test/Modules/cxx20-10-1-ex1.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-10-1-ex1.cpp
@@ -0,0 +1,50 @@
+// The example in the standard is not in required build order.
+// revised here
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN:  -o %t/A_Internals.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN:  -fmodule-file=%t/A_Internals.pcm -o %t/A_Foo.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=3 -x c++ %s \
+// RUN:  -fmodule-file=%t/A_Foo.pcm -o %t/A.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj  -D TU=4 -x c++ %s \
+// RUN:  -fmodule-file=%t/A.pcm -o %t/ex1.o
+
+// expected-no-diagnostics
+
+#if TU == 1
+
+module A:Internals;
+int bar();
+
+#elif TU == 2
+
+export module A:Foo;
+
+import :Internals;
+
+export int foo() { return 2 * (bar() + 1); }
+
+#elif TU == 3
+
+export module A;
+export import :Foo;
+export int baz();
+
+#elif TU == 4
+module A;
+
+import :Internals;
+
+int bar() { return baz() - 10; }
+int baz() { return 30; }
+
+#else
+#error "no TU set"
+#endif
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -110,9 +110,24 @@
   // module state;
   ImportState = ModuleImportState::NotACXX20Module;
 
-  // A module implementation unit requires that we are not compiling a module
-  // of any kind. A module interface unit requires that we are not compiling a
-  // module map.
+  bool IsPartition = !Partition.emp

[PATCH] D118587: [C++20][Modules][4/8] Handle generation of partition implementation CMIs.

[Iain Sandoe via Phabricator via cfe-commits]

iains added inline comments.



Comment at: clang/test/Modules/cxx20-10-1-ex1.cpp:7-8
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN:  -o %t/A_Internals.pcm
+

ChuanqiXu wrote:
> In my implementation, I replace ':' to '-' in pcm filename. I do so since I 
> remember this is the behavior of GCC. I think '-' is better than '_' since 
> '_' is possible to occur in name.
> 
> And I am surprised that the compiler wouldn't complain about that it couldn't 
> find the corresponding pcm. Since I don't see corresponding code so far.
The filename could/should be completely decoupled from the module name; 
changing the filenames to something randomly chosen should make no difference 
to the tests passing.

I made a comment on why/how this works in 3/8 in reply a similar comment made 
there (essentially the pairing of module-name and filename is something known 
by the lmodule loader and/or the header search mechanisms - it is not something 
that Sema needs to know).





Comment at: clang/test/Modules/cxx20-10-1-ex1.cpp:21
+
+#if TU == 1
+

urnathan wrote:
> I think I'm getting used to this idiom, or maybe Stockholm has kicked in?
we are all hostage to some style decisions made by others?
FAOD, the idiom is not my device, I plagiarised ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118587

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

[PATCH] D114425: [clang] Add __builtin_bswap128

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 409629.
philnik marked an inline comment as done.
philnik added a comment.
Herald added a subscriber: mstorsjo.

- Add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtin-bswap128.c
  clang/test/CodeGen/builtins.cpp
  clang/test/Sema/builtin-bswap128.c
  clang/test/Sema/constant-builtins-2.c

Index: clang/test/Sema/constant-builtins-2.c
===
--- clang/test/Sema/constant-builtins-2.c
+++ clang/test/Sema/constant-builtins-2.c
@@ -216,6 +216,9 @@
 int h3 = __builtin_bswap16(0x1234) == 0x3412 ? 1 : f();
 int h4 = __builtin_bswap32(0x1234) == 0x3412 ? 1 : f();
 int h5 = __builtin_bswap64(0x1234) == 0x3412 ? 1 : f();
+#if defined(__SIZEOF_INT128__)
+int h6 = __builtin_bswap128(0x1234) == (((__int128)0x3412) << 112) ? 1 : f();
+#endif
 extern long int bi0;
 extern __typeof__(__builtin_expect(0, 0)) bi0;
 
Index: clang/test/Sema/builtin-bswap128.c
===
--- /dev/null
+++ clang/test/Sema/builtin-bswap128.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple i386-mingw32 -fsyntax-only -verify %s
+
+void test() {
+  __builtin_bswap128(1); // expected-error {{use of unknown builtin '__builtin_bswap128'}}
+}
Index: clang/test/CodeGen/builtins.cpp
===
--- clang/test/CodeGen/builtins.cpp
+++ clang/test/CodeGen/builtins.cpp
@@ -20,6 +20,10 @@
 decltype(__builtin_bswap32(0)) bswap32 = 42;
 extern uint64_t bswap64;
 decltype(__builtin_bswap64(0)) bswap64 = 42;
+#ifdef __SIZEOF_INT128__
+extern __uint128_t bswap128;
+decltype(__builtin_bswap128(0)) bswap128 = 42;
+#endif
 
 #ifdef __clang__
 extern uint8_t bitrev8;
Index: clang/test/CodeGen/builtin-bswap128.c
===
--- /dev/null
+++ clang/test/CodeGen/builtin-bswap128.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: @test(
+// CHECK-NEXT: entry:
+// CHECK-NEXT:[[R1:%.*]] = alloca i128, align 16
+// CHECK-NEXT:[[R2:%.*]] = alloca i128, align 16
+// CHECK-NEXT:store i128 1329227995784915872903807060280344576, i128* [[R1:%.*]], align 16
+// CHECK-NEXT:store i128 2658455991569831745807614120560689152, i128* [[R2:%.*]], align 16
+void test() {
+  __int128 r1 = __builtin_bswap128(1);
+  __int128 r2 = __builtin_bswap128((__int128)2);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2929,7 +2929,8 @@
   }
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
-  case Builtin::BI__builtin_bswap64: {
+  case Builtin::BI__builtin_bswap64:
+  case Builtin::BI__builtin_bswap128: {
 return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::bswap));
   }
   case Builtin::BI__builtin_bitreverse8:
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11784,7 +11784,8 @@
 
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
-  case Builtin::BI__builtin_bswap64: {
+  case Builtin::BI__builtin_bswap64:
+  case Builtin::BI__builtin_bswap128: {
 APSInt Val;
 if (!EvaluateInteger(E->getArg(0), Val, Info))
   return false;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -10839,8 +10839,8 @@
 /// to be an Integer Constant Expression.
 static QualType DecodeTypeFromStr(const char *&Str, const ASTContext &Context,
   ASTContext::GetBuiltinTypeError &Error,
-  bool &RequiresICE,
-  bool AllowTypeModifiers) {
+  bool &RequiresICE, bool AllowTypeModifiers,
+  bool AllowInt128 = false) {
   // Modifiers.
   int HowLong = 0;
   bool Signed = false, Unsigned = false;
@@ -10983,9 +10983,13 @@
   Type = Context.ShortTy;
 break;
   case 'i':
-if (HowLong == 3)
+if (HowLong == 3) {
+  if (!AllowInt128 && !Context.getTargetInfo().hasInt128Type()) {
+Error = ASTContext::GE_Missing_type;
+return {};
+  }
   Type = Unsigned ? Context.UnsignedInt128Ty : Context.Int128Ty;
-else if (HowLong == 2)
+} else if (HowLong == 2)
   Type = Unsigned ? Context.UnsignedLongLongTy : Context.LongLongTy;
 else if (HowLong == 1)

[PATCH] D118588: [C++20][Modules]5/8] Diagnose wrong import/export for partition CMIs.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 409633.
iains added a comment.

rebased onto changes in parent patches


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118588

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaModule.cpp
  clang/test/Modules/cxx20-10-3-ex1.cpp
  clang/test/Modules/cxx20-10-3-ex2.cpp
  clang/test/Modules/cxx20-import-diagnostics-a.cpp

Index: clang/test/Modules/cxx20-import-diagnostics-a.cpp
===
--- clang/test/Modules/cxx20-import-diagnostics-a.cpp
+++ clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -117,13 +117,13 @@
 
 module B;
 
-import B; // expected-error {{import of module 'B' appears within same top-level module 'B'}}
+import B; // expected-error {{import of module 'B' appears within its own implementation}}
 
 #elif TU == 9
 
 export module B;
 
-import B; // expected-error {{import of module 'B' appears within same top-level module 'B'}}
+import B; // expected-error {{import of module 'B' appears within its own interface}}
 
 #elif TU == 10
 
Index: clang/test/Modules/cxx20-10-3-ex2.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-10-3-ex2.cpp
@@ -0,0 +1,28 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=0 -x c++ %s \
+// RUN:  -o %t/M.pcm
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=1 -x c++ %s \
+// RUN:  -fmodule-file=%t/M.pcm -o %t/tu_8.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN:  -o %t/M.pcm -verify
+
+#if TU == 0
+export module M;
+
+#elif TU == 1
+module M;
+  // error: cannot import M in its own unit
+import M; // expected-error {{import of module 'M' appears within its own implementation}}
+
+#elif TU == 2
+export module M;
+  // error: cannot import M in its own unit
+import M; // expected-error {{import of module 'M' appears within its own interface}}
+
+#else
+#error "no TU set"
+#endif
Index: clang/test/Modules/cxx20-10-3-ex1.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-10-3-ex1.cpp
@@ -0,0 +1,36 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN:  -o %t/M_PartImpl.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN:  -fmodule-file=%t/M_PartImpl.pcm -o %t/M.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=3 -x c++ %s \
+// RUN:  -o %t/M_Part.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=4 -x c++ %s \
+// RUN:  -fmodule-file=%t/M_Part.pcm -o %t/M.pcm
+
+#if TU == 1
+module M:PartImpl;
+
+// expected-no-diagnostics
+#elif TU == 2
+export module M;
+ // error: exported partition :Part is an implementation unit
+export import :PartImpl; // expected-error {{module partition implementations cannot be exported}}
+
+#elif TU == 3
+export module M:Part;
+
+// expected-no-diagnostics
+#elif TU == 4
+export module M;
+export import :Part;
+
+// expected-no-diagnostics
+#else
+#error "no TU set"
+#endif
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -403,10 +403,16 @@
   }
 
   // Diagnose self-import before attempting a load.
+  // [module.import]/9
+  // A module implementation unit of a module M that is not a module partition
+  // shall not contain a module-import-declaration nominating M.
+  // (for an implementation, the module interface is imported implicitly,
+  //  but that's handled in the module decl code).
+
   if (getLangOpts().CPlusPlusModules && isCurrentModulePurview() &&
   getCurrentModule()->Name == ModuleName) {
-Diag(ImportLoc, diag::err_module_self_import)
-<< ModuleName << getLangOpts().CurrentModule;
+Diag(ImportLoc, diag::err_module_self_import_cxx20)
+<< ModuleName << !ModuleScopes.back().ModuleInterface;
 return true;
   }
 
@@ -440,8 +446,7 @@
   // of the same top-level module. Until we do, make it an error rather than
   // silently ignoring the import.
   // FIXME: Should we warn on a redundant import of the current module?
-  if (!getLangOpts().CPlusPlusModules &&
-  Mod->getTopLevelModuleName() == getLangOpts().CurrentModule &&
+  if (Mod->getTopLevelModuleName() == getLangOpts().CurrentModule &&
   (getLangOpts().isCompilingModule() || !getLangOpts().ModulesTS)) {
 Diag(ImportLoc, getLangOpts().isCompilingModule()
 ? diag::err_module_self_import
@@ -482,7 +487,12 @@
   if (!ModuleScopes.empty())
 Context.addModuleInitializer(ModuleScopes.back().Module, Import);
 
-  if (!ModuleScopes.empty() && ModuleScopes.back().ModuleInterface) {
+  // A

[PATCH] D116774: AST: Move __va_list tag back to std conditionally on AArch64.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:641
+getASTContext().getTargetInfo().getTriple().getArch();
+if (Arch == llvm::Triple::aarch64 || Arch == llvm::Triple::aarch64_be ||
+Arch == llvm::Triple::arm || Arch == llvm::Triple::armeb)

Could use isAArch64 (which then also picks up aarch64_32)



Comment at: clang/lib/AST/ItaniumMangle.cpp:642
+if (Arch == llvm::Triple::aarch64 || Arch == llvm::Triple::aarch64_be ||
+Arch == llvm::Triple::arm || Arch == llvm::Triple::armeb)
+  return getStdNamespace();

Could use isARM. Does this also need to care about isThumb, or has that been 
normalised to Arm with a T32 subarch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116774

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

[PATCH] D120055: [asan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
glider added reviewers: melver, dvyukov, eugenis.
Herald added a subscriber: hiraditya.
glider requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

For ASan this will effectively serve as a synonym for
__attribute__((no_sanitize("address"))).

Adding the disable_sanitizer_instrumentation to functions will drop the
sanitize_XXX attributes on the IR level.

This is the third reland of https://reviews.llvm.org/D114421.
Now that TSan test is fixed (https://reviews.llvm.org/D120050) there
should be no deadlocks.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120055

Files:
  clang/docs/AddressSanitizer.rst
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/test/CodeGen/address-safety-attr-flavors.cpp
  clang/test/CodeGen/asan-globals.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  
llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll

Index: llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
@@ -0,0 +1,47 @@
+; This test checks that we are not instrumenting sanitizer code.
+; RUN: opt < %s -passes='asan-pipeline' -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function with sanitize_address is instrumented.
+; Function Attrs: nounwind uwtable
+define void @instr_sa(i32* %a) sanitize_address {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @instr_sa
+; CHECK: call void @__asan_report_load
+
+
+; Function with disable_sanitizer_instrumentation is not instrumented.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi(i32* %a) disable_sanitizer_instrumentation {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi
+; CHECK-NOT: call void @__asan_report_load
+
+
+; disable_sanitizer_instrumentation takes precedence over sanitize_address.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi_sa(i32* %a) disable_sanitizer_instrumentation sanitize_address {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi_sa
+; CHECK-NOT: call void @__asan_report_load
+
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2888,6 +2888,9 @@
   // Leave if the function doesn't need instrumentation.
   if (!F.hasFnAttribute(Attribute::SanitizeAddress)) return FunctionModified;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return FunctionModified;
+
   LLVM_DEBUG(dbgs() << "ASAN instrumenting:\n" << F << "\n");
 
   initializeCallbacks(*F.getParent());
Index: clang/test/CodeGen/asan-globals.cpp
===
--- clang/test/CodeGen/asan-globals.cpp
+++ clang/test/CodeGen/asan-globals.cpp
@@ -10,6 +10,7 @@
 int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
+int __attribute__((disable_sanitizer_instrumentation)) disable_instrumentation_global;
 int ignorelisted_global;
 
 int __attribute__((section("__DATA, __common"))) sectioned_global; // KASAN - ignore globals in a section
@@ -50,31 +51,33 @@
 // UWTABLE: attributes #[[#ATTR]] = { nounwind uwtable }
 // UWTABLE: ![[#]] = !{i32 7, !"uwtable", i32 2}
 
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[IGNORELISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[DISABLE_INSTR_GLOBAL:[0-9]+]], ![[IGNORELISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
 // CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, i1 false}
 // CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 10, i32 5}
 // CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], !"dyn_init_global",

[PATCH] D118587: [C++20][Modules][4/8] Handle generation of partition implementation CMIs.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 409637.
iains added a comment.

fix a typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118587

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaModule.cpp
  clang/test/Modules/cxx20-10-1-ex1.cpp
  clang/test/Modules/cxx20-10-1-ex2.cpp
  clang/test/Modules/cxx20-import-diagnostics-a.cpp

Index: clang/test/Modules/cxx20-import-diagnostics-a.cpp
===
--- clang/test/Modules/cxx20-import-diagnostics-a.cpp
+++ clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -94,9 +94,9 @@
 #elif TU == 6
 
 module;
-// We can only have preprocessor commands here, which could include an include
+// We can only have preprocessor directives here, which permits an include-
 // translated header unit.  However those are identified specifically by the
-// preprocessor; non-preprocessed user code should not contain an import here.
+// preprocessor; non-preprocessed user code should not contain an 'import' here.
 import B; // expected-error {{module imports cannot be in the global module fragment}}
 
 export module D;
Index: clang/test/Modules/cxx20-10-1-ex2.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-10-1-ex2.cpp
@@ -0,0 +1,60 @@
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN:  -o %t/B_Y.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN:  -fmodule-file=%t/B_Y.pcm -o %t/B.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=3 -x c++ %s \
+// RUN:   -o %t/B_X1.pcm -verify
+
+// Not expected to work yet.
+//  %clang_cc1 -std=c++20 -emit-module-interface -D TU=4 -x c++ %s \
+//   -fmodule-file=%t/B.pcm  -o %t/B_X2.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -D TU=5 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm  -o %t/b_tu5.s
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=6 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm  -o %t/b_tu6.s -verify
+
+// Not expected to work yet.
+//  %clang_cc1 -std=c++20 -emit-module-interface -D TU=7 -x c++ %s \
+//   -fmodule-file=%t/B_X2.pcm  -o %t/B_X3.pcm -verify
+
+#if TU == 1
+  module B:Y;
+  int y();
+// expected-no-diagnostics
+#elif TU == 2
+  export module B;
+  import :Y;
+  int n = y();
+// expected-no-diagnostics
+#elif TU == 3
+  module B:X1; // does not implicitly import B
+  int &a = n;  // expected-error {{use of undeclared identifier }}
+#elif TU == 4
+  module B:X2; // does not implicitly import B
+  import B;
+  int &b = n; // OK
+// expected-no-diagnostics
+#elif TU == 5
+  module B; // implicitly imports B
+  int &c = n; // OK
+// expected-no-diagnostics
+#elif TU == 6
+  import B;
+  // error, n is module-local and this is not a module.
+  int &c = n; // expected-error {{use of undeclared identifier}}
+#elif TU == 7
+  module B:X3; // does not implicitly import B
+  import :X2; // X2 is an implementation so exports nothing.
+  // error: n not visible here.
+  int &c = n; // expected-error {{use of undeclared identifier }}
+#else
+#error "no TU set"
+#endif
Index: clang/test/Modules/cxx20-10-1-ex1.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-10-1-ex1.cpp
@@ -0,0 +1,50 @@
+// The example in the standard is not in required build order.
+// revised here
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN:  -o %t/A_Internals.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN:  -fmodule-file=%t/A_Internals.pcm -o %t/A_Foo.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=3 -x c++ %s \
+// RUN:  -fmodule-file=%t/A_Foo.pcm -o %t/A.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj  -D TU=4 -x c++ %s \
+// RUN:  -fmodule-file=%t/A.pcm -o %t/ex1.o
+
+// expected-no-diagnostics
+
+#if TU == 1
+
+module A:Internals;
+int bar();
+
+#elif TU == 2
+
+export module A:Foo;
+
+import :Internals;
+
+export int foo() { return 2 * (bar() + 1); }
+
+#elif TU == 3
+
+export module A;
+export import :Foo;
+export int baz();
+
+#elif TU == 4
+module A;
+
+import :Internals;
+
+int bar() { return baz() - 10; }
+int baz() { return 30; }
+
+#else
+#error "no TU set"
+#endif
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -110,9 +110,24 @@
   // module state;
   ImportState = ModuleImportState::NotACXX20Module;
 
-  // A module implementation unit requires that we are not compiling a module
-  // of any kind. A module interface unit requires that we are not compiling a
-  // module map.
+  bool IsPartition = !Partition.empty();
+  if (IsPartition)
+switch (MDK) {
+case Modul

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682
+Expr::EvalResult EVRX, EVRY;
+if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) ||
+!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C))
+  return false;
+
+APValue VX = EVRX.Val, VY = EVRY.Val;
+if (VX.getKind() != VY.getKind())

urnathan wrote:
> ChuanqiXu wrote:
> > urnathan wrote:
> > > ChuanqiXu wrote:
> > > > urnathan wrote:
> > > > > I'm kind of surprised how complex this check is.  Isn't there an AST 
> > > > > comparator available somewhere?
> > > > I found ODRHash. I think it is much better now.
> > > hm that suggests there there must be a comparator too -- this isn't a 
> > > cryptographically strong hash is it?  How would the compiler currently 
> > > make use of 'definitely different' and 'probably the same' without such a 
> > > comparator?
> > Yeah, I am sure there is not an such comparator. Or it has some methods 
> > like: `ASTContext::hasSameType` for type and `ASTContext::isSameEntity()` 
> > for Decl. But it lacks such methods now for Stmt and Expr.
> > 
> > > How would the compiler currently make use of 'definitely different' and 
> > > 'probably the same' without such a comparator?
> > 
> > Now it uses the two methods I listed above and ODRHash to compare. I think 
> > the two methods works for  'definitely different' and ODRHash works for 
> > 'probably the same'. So it's the reason why my previous implementation 
> > looks lengthy. Since I want to handle it by hand. (The previous method only 
> > works for simple Expr. I think it would be large work to implement 
> > comparator for whole Expr or Stmt).
> Hm, how do template instantations work -- there must be some way of 
> determining 'is this instantation just here the same as one I've already seen'
also, the same functionality (with more generality) is needed for comparing 
regular function default arguments with multiple definitions in the GM.  How is 
that done (or is it yet to be implemented?)


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

https://reviews.llvm.org/D118034

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

[PATCH] D120034: [clang-format] PROPOSAL - WIP: Added ability to parse template arguments

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

A noble effort. It needs someone like yourself to do this as you already have a 
deep understanding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120034

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

[PATCH] D118350: [Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct member is passed to a function

[Sean Fertile via Phabricator via cfe-commits]

sfertile added a comment.

Hey Zarko. I really like how we only issue the warning when the struct is used, 
however I think this is still overly broad because of where the incompatibility 
lies between xlc and clang. I believe the layout of the structures will be the 
same for both compilers, and globals of this type will have the same alignment 
restrictions. A function like `void baz(int *);` will be compatible between the 
2 compilers since the argument is just a pointer. The difference occurs when 
passing the structure by value on the stack, where xlc doesn't align the struct 
to the expected alignment, and clang/llvm does.  Since the incompatibility is 
in the calling convention only when the struct is passed byval, that should be 
the only time we emit the diagnostic.

There are 2 other things we should verify since I'm not sure if they are 
compatible or not:

1. When passing these structures byval but in argument passing registers, I'm 
guessing that xlc doesn't skip registers whose image in the PSA is 
under-aligned while llvm will.
2. Whether xlc passes these misaligned when its done through va_args.

The answers to those 2 questions will determine if we need to warn for all 
byval arguments that have alignments from attribute that is greater than 8, or 
if we can limit it a bit further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118350

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

[PATCH] D118589: [C++20][Modules][6/8] Record direct module imports.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 409645.
iains added a comment.

rebased onto parent patch changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118589

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaModule.cpp


Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -513,6 +513,11 @@
 assert(ThisModule && "was expecting a module if building one");
   }
 
+  // In some cases we need to know if an entity was present in a directly-
+  // imported module (as opposed to a transitive import).  This avoids
+  // searching both Imports and Exports.
+  DirectModuleImports.insert(Mod);
+
   return Import;
 }
 
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -,6 +,9 @@
   /// The global module fragment of the current translation unit.
   clang::Module *GlobalModuleFragment = nullptr;
 
+  /// The modules we imported directly.
+  llvm::SmallPtrSet DirectModuleImports;
+
   /// Namespace definitions that we will export when they finish.
   llvm::SmallPtrSet DeferredExportedNamespaces;
 
@@ -2249,6 +2252,10 @@
 return Entity->getOwningModule();
   }
 
+  bool isModuleDirectlyImported(const Module *M) {
+return DirectModuleImports.contains(M);
+  }
+
   /// Make a merged definition of an existing hidden definition \p ND
   /// visible at the specified location.
   void makeMergedDefinitionVisible(NamedDecl *ND);


Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -513,6 +513,11 @@
 assert(ThisModule && "was expecting a module if building one");
   }
 
+  // In some cases we need to know if an entity was present in a directly-
+  // imported module (as opposed to a transitive import).  This avoids
+  // searching both Imports and Exports.
+  DirectModuleImports.insert(Mod);
+
   return Import;
 }
 
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -,6 +,9 @@
   /// The global module fragment of the current translation unit.
   clang::Module *GlobalModuleFragment = nullptr;
 
+  /// The modules we imported directly.
+  llvm::SmallPtrSet DirectModuleImports;
+
   /// Namespace definitions that we will export when they finish.
   llvm::SmallPtrSet DeferredExportedNamespaces;
 
@@ -2249,6 +2252,10 @@
 return Entity->getOwningModule();
   }
 
+  bool isModuleDirectlyImported(const Module *M) {
+return DirectModuleImports.contains(M);
+  }
+
   /// Make a merged definition of an existing hidden definition \p ND
   /// visible at the specified location.
   void makeMergedDefinitionVisible(NamedDecl *ND);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

In D91605#3329375 , @ro wrote:

> 



> As an experiment, I've tried to use `GetStaticTlsBoundary` instead.  It does 
> indeed work on recent Solaris 11.4 and the resulting patch is at D120048 
> .  I'm pretty certain that support for 
> Solaris 11.3/Illumos which lack `dlpi_tls_modid` using `dlinfo(RTLD_SELF, 
> RTLD_DI_LINKMAP)` can be added on top of that one, unbreaking the Illumos 
> build.  This would avoid considerable duplication of the `dl_iterate_phdr` 
> code, which is certainly a nice benefit.  I'll experiment with that route 
> later.

Now posted as D120059 .  It this approach is 
acceptable, the series of D119829 , D120048 
, and D120059 
 will supercede this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91605

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

[PATCH] D109239: Add support for floating-option `-ffp-eval-method` and for new `pragma clang fp eval-method`

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D109239#3326874 , @thakis wrote:

> Any idea what's going on in that Modules/cxx20-export-import.cpp failure?

@thakis I wasn't able to reproduce the issue with 
Modules/cxx20-export-import.cpp locally (tried a couple of build 
configurations). May be it was due to the warning missing the group?  What do 
you think about pushing this commit and if the issue arises again; we can 
revert the patch if the issue persists?


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

https://reviews.llvm.org/D109239

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

[PATCH] D120034: [clang-format] PROPOSAL - WIP: Added ability to parse template arguments

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

That would be a nice improvement... and a big undertaking at the same time. You 
have my blessing!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120034

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

[PATCH] D120061: [libclang] add support for arrays to clang_Cursor_Evaluate

[Bernhard Wodok via Phabricator via cfe-commits]

BartmanAbyss created this revision.
BartmanAbyss added reviewers: klimek, rsmith, akyrtzi.
BartmanAbyss added projects: clang-c, clang.
Herald added a subscriber: arphaman.
BartmanAbyss requested review of this revision.
Herald added a subscriber: cfe-commits.

This allows evaluating array initializer such as

  int array[4]{1, 2, 3, 4};
  const char* str[2]{ "hello", "world" };

Adds `CXEval_Array` to the `CXEvalResultKind` enum, 
`clang_EvalResult_getArraySize()`, `clang_EvalResult_getArrayElt`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120061

Files:
  clang/include/clang-c/Index.h
  clang/test/Index/evaluate-cursor.cpp
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3900,10 +3900,16 @@
 long long intVal;
 double floatVal;
 char *stringVal;
+struct {
+  ExprEvalResult *elts;
+  unsigned size;
+} arrayVal;
   } EvalData;
   bool IsUnsignedInt;
   ~ExprEvalResult() {
-if (EvalType != CXEval_UnExposed && EvalType != CXEval_Float &&
+if (EvalType == CXEval_Array) {
+  delete[] EvalData.arrayVal.elts;
+} else if (EvalType != CXEval_UnExposed && EvalType != CXEval_Float &&
 EvalType != CXEval_Int) {
   delete[] EvalData.stringVal;
 }
@@ -3964,51 +3970,75 @@
   return ((ExprEvalResult *)E)->EvalData.stringVal;
 }
 
-static const ExprEvalResult *evaluateExpr(Expr *expr, CXCursor C) {
-  Expr::EvalResult ER;
-  ASTContext &ctx = getCursorContext(C);
-  if (!expr)
-return nullptr;
+unsigned clang_EvalResult_getArraySize(CXEvalResult E) {
+  if (!E || ((ExprEvalResult *)E)->EvalType != CXEval_Array) {
+return 0;
+  }
+  return ((ExprEvalResult *)E)->EvalData.arrayVal.size;
+}
 
-  expr = expr->IgnoreParens();
-  if (expr->isValueDependent())
-return nullptr;
-  if (!expr->EvaluateAsRValue(ER, ctx))
-return nullptr;
+CXEvalResult clang_EvalResult_getArrayElt(CXEvalResult E, unsigned index) {
+  if (E && ((ExprEvalResult *)E)->EvalType == CXEval_Array &&
+  index < ((ExprEvalResult *)E)->EvalData.arrayVal.size) {
+return &((ExprEvalResult *)E)->EvalData.arrayVal.elts[index];
+  }
+  return nullptr;
+}
 
-  QualType rettype;
-  CallExpr *callExpr;
-  auto result = std::make_unique();
-  result->EvalType = CXEval_UnExposed;
-  result->IsUnsignedInt = false;
+static bool evaluateVal(const APValue &val, ExprEvalResult &result) {
+  result.EvalType = CXEval_UnExposed;
+  result.IsUnsignedInt = false;
+
+  if (val.isArray()) {
+result.EvalType = CXEval_Array;
+result.EvalData.arrayVal.size = val.getArrayInitializedElts();
+result.EvalData.arrayVal.elts =
+new ExprEvalResult[result.EvalData.arrayVal.size];
+bool ret = true;
+for (unsigned i = 0; i < result.EvalData.arrayVal.size; i++) {
+  if (!evaluateVal(val.getArrayInitializedElt(i),
+   result.EvalData.arrayVal.elts[i])) {
+ret = false;
+  }
+}
 
-  if (ER.Val.isInt()) {
-result->EvalType = CXEval_Int;
+return ret;
+  }
 
-auto &val = ER.Val.getInt();
-if (val.isUnsigned()) {
-  result->IsUnsignedInt = true;
-  result->EvalData.unsignedVal = val.getZExtValue();
+  if (val.isInt()) {
+result.EvalType = CXEval_Int;
+
+auto &ival = val.getInt();
+if (ival.isUnsigned()) {
+  result.IsUnsignedInt = true;
+  result.EvalData.unsignedVal = ival.getZExtValue();
 } else {
-  result->EvalData.intVal = val.getExtValue();
+  result.EvalData.intVal = ival.getExtValue();
 }
 
-return result.release();
+return true;
   }
 
-  if (ER.Val.isFloat()) {
+  if (val.isFloat()) {
 llvm::SmallVector Buffer;
-ER.Val.getFloat().toString(Buffer);
+val.getFloat().toString(Buffer);
 std::string floatStr(Buffer.data(), Buffer.size());
-result->EvalType = CXEval_Float;
+result.EvalType = CXEval_Float;
 bool ignored;
-llvm::APFloat apFloat = ER.Val.getFloat();
+llvm::APFloat apFloat = val.getFloat();
 apFloat.convert(llvm::APFloat::IEEEdouble(),
 llvm::APFloat::rmNearestTiesToEven, &ignored);
-result->EvalData.floatVal = apFloat.convertToDouble();
-return result.release();
+result.EvalData.floatVal = apFloat.convertToDouble();
+return true;
   }
 
+  return false;
+}
+
+static bool evaluateStringExpr(Expr *expr, ASTContext &ctx, ExprEvalResult &result) {
+  QualType rettype;
+  CallExpr *callExpr;
+
   if (expr->getStmtClass() == Stmt::ImplicitCastExprClass) {
 const ImplicitCastExpr *I = dyn_cast(expr);
 auto *subExpr = I->getSubExprAsWritten();
@@ -4020,18 +4050,18 @@
 
   if (ObjCExpr) {
 StrE = ObjCExpr->getString();
-result->EvalType = CXEval_ObjCStrLiteral;
+result.EvalType = CXEval_ObjCStrLiteral;
   } else {
 StrE = cast(I-

[PATCH] D109239: Add support for floating-option `-ffp-eval-method` and for new `pragma clang fp eval-method`

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Sure, sounds good.


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

https://reviews.llvm.org/D109239

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

[PATCH] D117795: [AArch64] Add some missing strict FP vector lowering

[John Brawn via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8e17c9613f36: [AArch64] Add some missing strict FP vector 
lowering (authored by john.brawn).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117795

Files:
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/test/CodeGen/AArch64/fp-intrinsics-vector.ll

Index: llvm/test/CodeGen/AArch64/fp-intrinsics-vector.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/fp-intrinsics-vector.ll
@@ -0,0 +1,886 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=aarch64-none-eabi %s -disable-strictnode-mutation -o - | FileCheck %s
+; RUN: llc -mtriple=aarch64-none-eabi -global-isel=true -global-isel-abort=2 -disable-strictnode-mutation %s -o - | FileCheck %s
+
+; Check that constrained fp vector intrinsics are correctly lowered.
+
+
+; Single-precision intrinsics
+
+define <4 x float> @add_v4f32(<4 x float> %x, <4 x float> %y) #0 {
+; CHECK-LABEL: add_v4f32:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:fadd v0.4s, v0.4s, v1.4s
+; CHECK-NEXT:ret
+  %val = call <4 x float> @llvm.experimental.constrained.fadd.v4f32(<4 x float> %x, <4 x float> %y, metadata !"round.tonearest", metadata !"fpexcept.strict") #0
+  ret <4 x float> %val
+}
+
+define <4 x float> @sub_v4f32(<4 x float> %x, <4 x float> %y) #0 {
+; CHECK-LABEL: sub_v4f32:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:fsub v0.4s, v0.4s, v1.4s
+; CHECK-NEXT:ret
+  %val = call <4 x float> @llvm.experimental.constrained.fsub.v4f32(<4 x float> %x, <4 x float> %y, metadata !"round.tonearest", metadata !"fpexcept.strict") #0
+  ret <4 x float> %val
+}
+
+define <4 x float> @mul_v4f32(<4 x float> %x, <4 x float> %y) #0 {
+; CHECK-LABEL: mul_v4f32:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:fmul v0.4s, v0.4s, v1.4s
+; CHECK-NEXT:ret
+  %val = call <4 x float> @llvm.experimental.constrained.fmul.v4f32(<4 x float> %x, <4 x float> %y, metadata !"round.tonearest", metadata !"fpexcept.strict") #0
+  ret <4 x float> %val
+}
+
+define <4 x float> @div_v4f32(<4 x float> %x, <4 x float> %y) #0 {
+; CHECK-LABEL: div_v4f32:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:fdiv v0.4s, v0.4s, v1.4s
+; CHECK-NEXT:ret
+  %val = call <4 x float> @llvm.experimental.constrained.fdiv.v4f32(<4 x float> %x, <4 x float> %y, metadata !"round.tonearest", metadata !"fpexcept.strict") #0
+  ret <4 x float> %val
+}
+
+define <4 x float> @fma_v4f32(<4 x float> %x, <4 x float> %y, <4 x float> %z) #0 {
+; CHECK-LABEL: fma_v4f32:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:fmla v2.4s, v1.4s, v0.4s
+; CHECK-NEXT:mov v0.16b, v2.16b
+; CHECK-NEXT:ret
+  %val = call <4 x float> @llvm.experimental.constrained.fma.v4f32(<4 x float> %x, <4 x float> %y, <4 x float> %z, metadata !"round.tonearest", metadata !"fpexcept.strict") #0
+  ret <4 x float> %val
+}
+
+define <4 x i32> @fptosi_v4i32_v4f32(<4 x float> %x) #0 {
+; CHECK-LABEL: fptosi_v4i32_v4f32:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:fcvtzs v0.4s, v0.4s
+; CHECK-NEXT:ret
+  %val = call <4 x i32> @llvm.experimental.constrained.fptosi.v4i32.v4f32(<4 x float> %x, metadata !"fpexcept.strict") #0
+  ret <4 x i32> %val
+}
+
+define <4 x i32> @fptoui_v4i32_v4f32(<4 x float> %x) #0 {
+; CHECK-LABEL: fptoui_v4i32_v4f32:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:fcvtzu v0.4s, v0.4s
+; CHECK-NEXT:ret
+  %val = call <4 x i32> @llvm.experimental.constrained.fptoui.v4i32.v4f32(<4 x float> %x, metadata !"fpexcept.strict") #0
+  ret <4 x i32> %val
+}
+
+define <4 x i64> @fptosi_v4i64_v4f32(<4 x float> %x) #0 {
+; CHECK-LABEL: fptosi_v4i64_v4f32:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:ext v1.16b, v0.16b, v0.16b, #8
+; CHECK-NEXT:fcvtl v0.2d, v0.2s
+; CHECK-NEXT:fcvtl v1.2d, v1.2s
+; CHECK-NEXT:fcvtzs v0.2d, v0.2d
+; CHECK-NEXT:fcvtzs v1.2d, v1.2d
+; CHECK-NEXT:ret
+  %val = call <4 x i64> @llvm.experimental.constrained.fptosi.v4i64.v4f32(<4 x float> %x, metadata !"fpexcept.strict") #0
+  ret <4 x i64> %val
+}
+
+define <4 x i64> @fptoui_v4i64_v4f32(<4 x float> %x) #0 {
+; CHECK-LABEL: fptoui_v4i64_v4f32:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:ext v1.16b, v0.16b, v0.16b, #8
+; CHECK-NEXT:fcvtl v0.2d, v0.2s
+; CHECK-NEXT:fcvtl v1.2d, v1.2s
+; CHECK-NEXT:fcvtzu v0.2d, v0.2d
+; CHECK-NEXT:fcvtzu v1.2d, v1.2d
+; CHECK-NEXT:ret
+  %val = call <4 x i64> @llvm.experimental.constrained.fptoui.v4i64.v4f32(<4 x float> %x, metadata !"fpexcept.strict") #0
+  ret <4 x i64> %val
+}
+
+define <4 x float> @sitofp_v4f32_v4i32(<4 x i32> %x) #0 {
+; CHECK-LABEL: sitofp_v4f32_v4i32:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:scvtf v0.4s, v0.4s
+; CHECK-NEXT:ret
+  %val = call <4 x float> @llvm.experimental.constrained.sitofp.v4f32.v4i32(<4 x i32> %x, metadata !"roun

[PATCH] D118598: [C++20][Modules][7/8] Find the primary interface name for a module.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 409665.
iains added a comment.

address review comments, rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118598

Files:
  clang/include/clang/Basic/Module.h
  clang/lib/Sema/SemaModule.cpp


Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -382,17 +382,10 @@
 // We already checked that we are in a module purview in the parser.
 assert(!ModuleScopes.empty() && "in a module purview, but no module?");
 Module *NamedMod = ModuleScopes.back().Module;
-if (ModuleScopes.back().IsPartition) {
-  // We're importing a partition into a partition, find the name of the
-  // owning named module.
-  size_t P = NamedMod->Name.find_first_of(":");
-  ModuleName = NamedMod->Name.substr(0, P + 1);
-} else {
-  // We're importing a partition into the named module itself (either the
-  // interface or an implementation TU).
-  ModuleName = NamedMod->Name;
-  ModuleName += ":";
-}
+// If we are importing into a partition, find the owning named module,
+// otherwise, the name of the importing named module.
+ModuleName = NamedMod->getPrimaryModuleInterfaceName().str();
+ModuleName += ":";
 ModuleName += stringFromPath(Partition);
 ModuleNameLoc = {PP.getIdentifierInfo(ModuleName), Partition[0].second};
 Partition = ModuleIdPath(ModuleNameLoc);
Index: clang/include/clang/Basic/Module.h
===
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -517,6 +517,17 @@
   /// Is this a module partition.
   bool isModulePartition() const { return Name.find(':') != std::string::npos; 
}
 
+  /// Get the primary module interface name from a partition.
+
+  StringRef getPrimaryModuleInterfaceName() const {
+if (Kind == ModulePartitionInterface ||
+Kind == ModulePartitionImplementation) {
+  auto pos = Name.find(':');
+  return StringRef(Name.data(), pos);
+}
+return StringRef(Name);
+  }
+
   /// Retrieve the full name of this module, including the path from
   /// its top-level module.
   /// \param AllowStringLiterals If \c true, components that might not be


Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -382,17 +382,10 @@
 // We already checked that we are in a module purview in the parser.
 assert(!ModuleScopes.empty() && "in a module purview, but no module?");
 Module *NamedMod = ModuleScopes.back().Module;
-if (ModuleScopes.back().IsPartition) {
-  // We're importing a partition into a partition, find the name of the
-  // owning named module.
-  size_t P = NamedMod->Name.find_first_of(":");
-  ModuleName = NamedMod->Name.substr(0, P + 1);
-} else {
-  // We're importing a partition into the named module itself (either the
-  // interface or an implementation TU).
-  ModuleName = NamedMod->Name;
-  ModuleName += ":";
-}
+// If we are importing into a partition, find the owning named module,
+// otherwise, the name of the importing named module.
+ModuleName = NamedMod->getPrimaryModuleInterfaceName().str();
+ModuleName += ":";
 ModuleName += stringFromPath(Partition);
 ModuleNameLoc = {PP.getIdentifierInfo(ModuleName), Partition[0].second};
 Partition = ModuleIdPath(ModuleNameLoc);
Index: clang/include/clang/Basic/Module.h
===
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -517,6 +517,17 @@
   /// Is this a module partition.
   bool isModulePartition() const { return Name.find(':') != std::string::npos; }
 
+  /// Get the primary module interface name from a partition.
+
+  StringRef getPrimaryModuleInterfaceName() const {
+if (Kind == ModulePartitionInterface ||
+Kind == ModulePartitionImplementation) {
+  auto pos = Name.find(':');
+  return StringRef(Name.data(), pos);
+}
+return StringRef(Name);
+  }
+
   /// Retrieve the full name of this module, including the path from
   /// its top-level module.
   /// \param AllowStringLiterals If \c true, components that might not be
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118598: [C++20][Modules][7/8] Find the primary interface name for a module.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 409666.
iains marked an inline comment as done.
iains added a comment.

remove stray blank line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118598

Files:
  clang/include/clang/Basic/Module.h
  clang/lib/Sema/SemaModule.cpp


Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -382,17 +382,10 @@
 // We already checked that we are in a module purview in the parser.
 assert(!ModuleScopes.empty() && "in a module purview, but no module?");
 Module *NamedMod = ModuleScopes.back().Module;
-if (ModuleScopes.back().IsPartition) {
-  // We're importing a partition into a partition, find the name of the
-  // owning named module.
-  size_t P = NamedMod->Name.find_first_of(":");
-  ModuleName = NamedMod->Name.substr(0, P + 1);
-} else {
-  // We're importing a partition into the named module itself (either the
-  // interface or an implementation TU).
-  ModuleName = NamedMod->Name;
-  ModuleName += ":";
-}
+// If we are importing into a partition, find the owning named module,
+// otherwise, the name of the importing named module.
+ModuleName = NamedMod->getPrimaryModuleInterfaceName().str();
+ModuleName += ":";
 ModuleName += stringFromPath(Partition);
 ModuleNameLoc = {PP.getIdentifierInfo(ModuleName), Partition[0].second};
 Partition = ModuleIdPath(ModuleNameLoc);
Index: clang/include/clang/Basic/Module.h
===
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -517,6 +517,16 @@
   /// Is this a module partition.
   bool isModulePartition() const { return Name.find(':') != std::string::npos; 
}
 
+  /// Get the primary module interface name from a partition.
+  StringRef getPrimaryModuleInterfaceName() const {
+if (Kind == ModulePartitionInterface ||
+Kind == ModulePartitionImplementation) {
+  auto pos = Name.find(':');
+  return StringRef(Name.data(), pos);
+}
+return StringRef(Name);
+  }
+
   /// Retrieve the full name of this module, including the path from
   /// its top-level module.
   /// \param AllowStringLiterals If \c true, components that might not be


Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -382,17 +382,10 @@
 // We already checked that we are in a module purview in the parser.
 assert(!ModuleScopes.empty() && "in a module purview, but no module?");
 Module *NamedMod = ModuleScopes.back().Module;
-if (ModuleScopes.back().IsPartition) {
-  // We're importing a partition into a partition, find the name of the
-  // owning named module.
-  size_t P = NamedMod->Name.find_first_of(":");
-  ModuleName = NamedMod->Name.substr(0, P + 1);
-} else {
-  // We're importing a partition into the named module itself (either the
-  // interface or an implementation TU).
-  ModuleName = NamedMod->Name;
-  ModuleName += ":";
-}
+// If we are importing into a partition, find the owning named module,
+// otherwise, the name of the importing named module.
+ModuleName = NamedMod->getPrimaryModuleInterfaceName().str();
+ModuleName += ":";
 ModuleName += stringFromPath(Partition);
 ModuleNameLoc = {PP.getIdentifierInfo(ModuleName), Partition[0].second};
 Partition = ModuleIdPath(ModuleNameLoc);
Index: clang/include/clang/Basic/Module.h
===
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -517,6 +517,16 @@
   /// Is this a module partition.
   bool isModulePartition() const { return Name.find(':') != std::string::npos; }
 
+  /// Get the primary module interface name from a partition.
+  StringRef getPrimaryModuleInterfaceName() const {
+if (Kind == ModulePartitionInterface ||
+Kind == ModulePartitionImplementation) {
+  auto pos = Name.find(':');
+  return StringRef(Name.data(), pos);
+}
+return StringRef(Name);
+  }
+
   /// Retrieve the full name of this module, including the path from
   /// its top-level module.
   /// \param AllowStringLiterals If \c true, components that might not be
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118333: [RISCV] Use computeTargetABI from llc as well as clang

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: llvm/test/CodeGen/RISCV/double-mem.ll:2
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv32 -mattr=+d -verify-machineinstrs < %s \
+; RUN: llc -mtriple=riscv32 -mattr=+d -target-abi=ilp32 -verify-machineinstrs 
< %s \
 ; RUN:   | FileCheck -check-prefix=RV32IFD %s

For a bunch of these it seems it'd make more sense to just use a hard-float 
ABI. I think it's worthwhile keeping this as NFC test-wise, but don't know if 
it makes sense to update the ABIs first and make this patch smaller or land 
this then go through at a later date so as to not stall this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118333

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

[PATCH] D118598: [C++20][Modules][7/8] Find the primary interface name for a module.

[Iain Sandoe via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/Module.h:523-524
+  std::string getPrimaryModuleInterfaceName() const {
+std::string::size_type pos = Name.find(':');
+if (pos == std::string::npos)
+  return Name;

iains wrote:
> iains wrote:
> > urnathan wrote:
> > > haven't you added an IsPartition flag to Module? can't you test that 
> > > before searching? Also, Almost Always Auto?
> > 1) The case we are dealing with is
> > 
> > we're building `A{,:B}`
> > we encounter `:Part`
> > 
> > So we do not have a module for the importer ('cos we didn't build it yet) 
> > but we need to know the primary module name for the imported partition 
> > (even to be able to find it).
> > 
> > So it's a wee bit icky, but we are reduced to manipulating text (none of 
> > the content of Module.h is available - we are looking at parser tokens).
> > 
> > 2) auto and StringRefs - yes probably I could do better.
> > 
> > The consolation is that this is not an action we'd reasonably expect to be 
> > carried out many times c.f. other parser jobs - of course, I'll be proven 
> > wrong and someone will import 10^6 partitions 
> > 
>  
> > (none of the content of Module.h is available - we are looking at parser 
> > tokens).
> 
> maybe I retract that, at least partially,  we do not have a module (built) - 
> but some of the base information is saved in sema when the module decl is 
> built, so we probably do know if we are building `A:B` or just `A`. 
> 
> If we think that this could be a pain point - sema can cache the primary 
> module name and we can then return a StringRef to that.
I've reworked this to use a StringRef - so that the source string components 
are owned by the importing module (which we're building) and then we construct 
a new module name (for the imported module) which we will use to find it.  We 
have enough of the module at this point to do this.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118598

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

[PATCH] D120065: [clang][SemaTemplate] Fix a stack use after scope

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
kadircet 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/D120065

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/lib/AST/DeclTemplate.cpp
  clang/test/SemaTemplate/friend-template.cpp


Index: clang/test/SemaTemplate/friend-template.cpp
===
--- clang/test/SemaTemplate/friend-template.cpp
+++ clang/test/SemaTemplate/friend-template.cpp
@@ -329,3 +329,12 @@
 foo(b); // expected-note {{in instantiation}}
   }
 }
+
+namespace StackUseAfterScope {
+template  class Bar {};
+class Foo {
+  // Make sure this doesn't crash.
+  template <> friend class Bar; // expected-error {{template 
specialization declaration cannot be a friend}}
+  bool aux;
+};
+}
Index: clang/lib/AST/DeclTemplate.cpp
===
--- clang/lib/AST/DeclTemplate.cpp
+++ clang/lib/AST/DeclTemplate.cpp
@@ -28,6 +28,7 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -1098,7 +1099,15 @@
SourceLocation L,
MutableArrayRef Params,
FriendUnion Friend, SourceLocation FLoc) {
-  return new (Context, DC) FriendTemplateDecl(DC, L, Params, Friend, FLoc);
+  TemplateParameterList **TPL = nullptr;
+  unsigned NumParams = 0;
+  if (!Params.empty()) {
+NumParams = Params.size();
+TPL = new (Context) TemplateParameterList *[NumParams];
+llvm::copy(Params, TPL);
+  }
+  return new (Context, DC)
+  FriendTemplateDecl(DC, L, TPL, NumParams, Friend, FLoc);
 }
 
 FriendTemplateDecl *FriendTemplateDecl::CreateDeserialized(ASTContext &C,
Index: clang/include/clang/AST/DeclTemplate.h
===
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -2461,10 +2461,10 @@
   SourceLocation FriendLoc;
 
   FriendTemplateDecl(DeclContext *DC, SourceLocation Loc,
- MutableArrayRef Params,
+ TemplateParameterList **Params, unsigned NumParams,
  FriendUnion Friend, SourceLocation FriendLoc)
-  : Decl(Decl::FriendTemplate, DC, Loc), NumParams(Params.size()),
-Params(Params.data()), Friend(Friend), FriendLoc(FriendLoc) {}
+  : Decl(Decl::FriendTemplate, DC, Loc), NumParams(NumParams),
+Params(Params), Friend(Friend), FriendLoc(FriendLoc) {}
 
   FriendTemplateDecl(EmptyShell Empty) : Decl(Decl::FriendTemplate, Empty) {}
 


Index: clang/test/SemaTemplate/friend-template.cpp
===
--- clang/test/SemaTemplate/friend-template.cpp
+++ clang/test/SemaTemplate/friend-template.cpp
@@ -329,3 +329,12 @@
 foo(b); // expected-note {{in instantiation}}
   }
 }
+
+namespace StackUseAfterScope {
+template  class Bar {};
+class Foo {
+  // Make sure this doesn't crash.
+  template <> friend class Bar; // expected-error {{template specialization declaration cannot be a friend}}
+  bool aux;
+};
+}
Index: clang/lib/AST/DeclTemplate.cpp
===
--- clang/lib/AST/DeclTemplate.cpp
+++ clang/lib/AST/DeclTemplate.cpp
@@ -28,6 +28,7 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -1098,7 +1099,15 @@
SourceLocation L,
MutableArrayRef Params,
FriendUnion Friend, SourceLocation FLoc) {
-  return new (Context, DC) FriendTemplateDecl(DC, L, Params, Friend, FLoc);
+  TemplateParameterList **TPL = nullptr;
+  unsigned NumParams = 0;
+  if (!Params.empty()) {
+NumParams = Params.size();
+TPL = new (Context) TemplateParameterList *[NumParams];
+llvm::copy(Params, TPL);
+  }
+  return new (Context, DC)
+  FriendTemplateDecl(DC, L, TPL, NumParams, Friend, FLoc);
 }
 
 FriendTemplateDecl *FriendTemplateDecl::CreateDeserialized(ASTContext &C,
Index: clang/include/clang/AST/DeclTemplate.h
===
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -2461,10 +2461,10 @@
   SourceLocation FriendLoc;
 
   FriendTemplateDecl(DeclContext *DC, SourceLocation Loc,
- MutableArrayRef Params,
+ TemplateParameterList **Params, unsigned NumParams,
  FriendUnion Friend, Sour

[PATCH] D120066: FileCheck’s regexp stumped on Windows.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam created this revision.
zahiraam added a reviewer: erichkeane.
zahiraam requested review of this revision.
Herald added a project: clang.

We found that on Windows the regular expression used to describe the link 
command from the last RUN command of the LIT test debug-info-hotpatch.cpp, is 
not matching the link command.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120066

Files:
  clang/test/CodeGenCXX/debug-info-hotpatch.cpp


Index: clang/test/CodeGenCXX/debug-info-hotpatch.cpp
===
--- clang/test/CodeGenCXX/debug-info-hotpatch.cpp
+++ clang/test/CodeGenCXX/debug-info-hotpatch.cpp
@@ -12,8 +12,8 @@
 // RUN: %clang_cl --target=x86_64-windows-msvc /hotpatch -### -- %s 2>&1 \
 // RUN:| FileCheck %s --check-prefix=FUNCTIONPADMIN
 // FUNCTIONPADMIN: clang{{.*}}
-// FUNCTIONPADMIN: {{link.*"}}
-// FUNCTIONPADMIN: -functionpadmin
+// FUNCTIONPADMIN: {{link[^"]*"}} 
+// FUNCTIONPADMIN: "-functionpadmin"
 
 int main() {
   return 0;


Index: clang/test/CodeGenCXX/debug-info-hotpatch.cpp
===
--- clang/test/CodeGenCXX/debug-info-hotpatch.cpp
+++ clang/test/CodeGenCXX/debug-info-hotpatch.cpp
@@ -12,8 +12,8 @@
 // RUN: %clang_cl --target=x86_64-windows-msvc /hotpatch -### -- %s 2>&1 \
 // RUN:| FileCheck %s --check-prefix=FUNCTIONPADMIN
 // FUNCTIONPADMIN: clang{{.*}}
-// FUNCTIONPADMIN: {{link.*"}}
-// FUNCTIONPADMIN: -functionpadmin
+// FUNCTIONPADMIN: {{link[^"]*"}} 
+// FUNCTIONPADMIN: "-functionpadmin"
 
 int main() {
   return 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120065: [clang][SemaTemplate] Fix a stack use after scope

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/AST/DeclTemplate.h:2464
   FriendTemplateDecl(DeclContext *DC, SourceLocation Loc,
- MutableArrayRef Params,
+ TemplateParameterList **Params, unsigned NumParams,
  FriendUnion Friend, SourceLocation FriendLoc)

Is this change just to emphasize that this constructor won't copy the contents?



Comment at: clang/lib/AST/DeclTemplate.cpp:1103
+  TemplateParameterList **TPL = nullptr;
+  unsigned NumParams = 0;
+  if (!Params.empty()) {

just initialize to params.size already? or inline this variable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120065

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

[PATCH] D118599: [C++20][Modules][8/8] Amend module visibility rules for partitions.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 409671.
iains added a comment.

rebase onto parent patch changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118599

Files:
  clang/lib/Sema/SemaLookup.cpp
  clang/test/Modules/cxx20-10-1-ex2.cpp


Index: clang/test/Modules/cxx20-10-1-ex2.cpp
===
--- clang/test/Modules/cxx20-10-1-ex2.cpp
+++ clang/test/Modules/cxx20-10-1-ex2.cpp
@@ -11,9 +11,8 @@
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=3 -x c++ %s \
 // RUN:   -o %t/B_X1.pcm -verify
 
-// Not expected to work yet.
-//  %clang_cc1 -std=c++20 -emit-module-interface -D TU=4 -x c++ %s \
-//   -fmodule-file=%t/B.pcm  -o %t/B_X2.pcm
+// RUN:  %clang_cc1 -std=c++20 -emit-module-interface -D TU=4 -x c++ %s \
+// RUN:   -fmodule-file=%t/B.pcm  -o %t/B_X2.pcm
 
 // RUN: %clang_cc1 -std=c++20 -emit-obj -D TU=5 -x c++ %s \
 // RUN:  -fmodule-file=%t/B.pcm  -o %t/b_tu5.s
@@ -21,9 +20,8 @@
 // RUN: %clang_cc1 -std=c++20 -S -D TU=6 -x c++ %s \
 // RUN:  -fmodule-file=%t/B.pcm  -o %t/b_tu6.s -verify
 
-// Not expected to work yet.
-//  %clang_cc1 -std=c++20 -emit-module-interface -D TU=7 -x c++ %s \
-//   -fmodule-file=%t/B_X2.pcm  -o %t/B_X3.pcm -verify
+// RUN:  %clang_cc1 -std=c++20 -emit-module-interface -D TU=7 -x c++ %s \
+// RUN: -fmodule-file=%t/B_X2.pcm  -o %t/B_X3.pcm -verify
 
 #if TU == 1
   module B:Y;
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1687,8 +1687,8 @@
   Module *DeclModule = SemaRef.getOwningModule(D);
   assert(DeclModule && "hidden decl has no owning module");
 
-  // If the owning module is visible, the decl is visible.
   if (SemaRef.isModuleVisible(DeclModule, D->isModulePrivate()))
+// If the owning module is visible, the decl is visible.
 return true;
 
   // Determine whether a decl context is a file context for the purpose of
@@ -1762,6 +1762,22 @@
   if (ModulePrivate) {
 if (isInCurrentModule(M, getLangOpts()))
   return true;
+else if (M->Kind == Module::ModuleKind::ModulePartitionImplementation &&
+ isModuleDirectlyImported(M))
+  // Unless a partition implementation is directly imported it is not
+  // counted as visible for lookup, although the contained decls might
+  // still be reachable.  It's a partition, so it must be part of the
+  // current module to be a valid import.
+  return true;
+else if (getLangOpts().CPlusPlusModules && !ModuleScopes.empty() &&
+ ModuleScopes[0].Module->Kind ==
+ Module::ModuleKind::ModulePartitionImplementation &&
+ ModuleScopes[0].Module->getPrimaryModuleInterfaceName() ==
+ M->Name &&
+ isModuleDirectlyImported(M))
+  // We are building a module implementation partition and the TU imports
+  // the primary module interface unit.
+  return true;
   } else {
 if (VisibleModules.isVisible(M))
   return true;


Index: clang/test/Modules/cxx20-10-1-ex2.cpp
===
--- clang/test/Modules/cxx20-10-1-ex2.cpp
+++ clang/test/Modules/cxx20-10-1-ex2.cpp
@@ -11,9 +11,8 @@
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=3 -x c++ %s \
 // RUN:   -o %t/B_X1.pcm -verify
 
-// Not expected to work yet.
-//  %clang_cc1 -std=c++20 -emit-module-interface -D TU=4 -x c++ %s \
-//   -fmodule-file=%t/B.pcm  -o %t/B_X2.pcm
+// RUN:  %clang_cc1 -std=c++20 -emit-module-interface -D TU=4 -x c++ %s \
+// RUN:   -fmodule-file=%t/B.pcm  -o %t/B_X2.pcm
 
 // RUN: %clang_cc1 -std=c++20 -emit-obj -D TU=5 -x c++ %s \
 // RUN:  -fmodule-file=%t/B.pcm  -o %t/b_tu5.s
@@ -21,9 +20,8 @@
 // RUN: %clang_cc1 -std=c++20 -S -D TU=6 -x c++ %s \
 // RUN:  -fmodule-file=%t/B.pcm  -o %t/b_tu6.s -verify
 
-// Not expected to work yet.
-//  %clang_cc1 -std=c++20 -emit-module-interface -D TU=7 -x c++ %s \
-//   -fmodule-file=%t/B_X2.pcm  -o %t/B_X3.pcm -verify
+// RUN:  %clang_cc1 -std=c++20 -emit-module-interface -D TU=7 -x c++ %s \
+// RUN: -fmodule-file=%t/B_X2.pcm  -o %t/B_X3.pcm -verify
 
 #if TU == 1
   module B:Y;
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1687,8 +1687,8 @@
   Module *DeclModule = SemaRef.getOwningModule(D);
   assert(DeclModule && "hidden decl has no owning module");
 
-  // If the owning module is visible, the decl is visible.
   if (SemaRef.isModuleVisible(DeclModule, D->isModulePrivate()))
+// If the owning module is visible, the decl is visible.
 return true;
 
   // Determine whether a decl context is a file context for the purpose of
@@ -1762,6 +1762,22 @@
   if (ModulePrivate) {
 if (isInCurrentModule(M, getLangOpts()))
   

[clang] 0b57e6c - [objcopy] followup patch after f75da0c8e65cf1b09012a8b62cd7f3e9a646bbc9

[Alexey Lapshin via cfe-commits]

Author: Alexey Lapshin
Date: 2022-02-17T19:32:10+03:00
New Revision: 0b57e6c46b707c0e7a123efe82abf3c1e7b5a503

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

LOG: [objcopy] followup patch after f75da0c8e65cf1b09012a8b62cd7f3e9a646bbc9

Added: 


Modified: 
clang/docs/tools/clang-formatted-files.txt
llvm/unittests/CMakeLists.txt

Removed: 




diff  --git a/clang/docs/tools/clang-formatted-files.txt 
b/clang/docs/tools/clang-formatted-files.txt
index 4a2c28bcf32ae..231877144d28a 100644
--- a/clang/docs/tools/clang-formatted-files.txt
+++ b/clang/docs/tools/clang-formatted-files.txt
@@ -5063,6 +5063,18 @@ llvm/include/llvm/MCA/Stages/InstructionTables.h
 llvm/include/llvm/MCA/Stages/MicroOpQueueStage.h
 llvm/include/llvm/MCA/Stages/RetireStage.h
 llvm/include/llvm/MCA/Stages/Stage.h
+llvm/include/llvm/ObjCopy/MultiFormatConfig.h
+llvm/include/llvm/ObjCopy/ConfigManager.h
+llvm/include/llvm/ObjCopy/CommonConfig.h
+llvm/include/llvm/ObjCopy/ObjCopy.h
+llvm/include/llvm/ObjCopy/wasm/WasmConfig.h
+llvm/include/llvm/ObjCopy/wasm/WasmObjcopy.h
+llvm/include/llvm/ObjCopy/ELF/ELFConfig.h
+llvm/include/llvm/ObjCopy/ELF/ELFObjcopy.h
+llvm/include/llvm/ObjCopy/MachO/MachOObjcopy.h
+llvm/include/llvm/ObjCopy/MachO/MachOConfig.h
+llvm/include/llvm/ObjCopy/COFF/COFFConfig.h
+llvm/include/llvm/ObjCopy/COFF/COFFObjcopy.h
 llvm/include/llvm/Object/Archive.h
 llvm/include/llvm/Object/COFFModuleDefinition.h
 llvm/include/llvm/Object/Decompressor.h
@@ -5766,6 +5778,34 @@ llvm/lib/MCA/Stages/InOrderIssueStage.cpp
 llvm/lib/MCA/Stages/MicroOpQueueStage.cpp
 llvm/lib/MCA/Stages/RetireStage.cpp
 llvm/lib/MCA/Stages/Stage.cpp
+llvm/lib/ObjCopy/Archive.cpp
+llvm/lib/ObjCopy/ConfigManager.cpp
+llvm/lib/ObjCopy/ObjCopy.cpp
+llvm/lib/ObjCopy/Archive.h
+llvm/lib/ObjCopy/wasm/Reader.cpp
+llvm/lib/ObjCopy/wasm/Reader.h
+llvm/lib/ObjCopy/wasm/Object.cpp
+llvm/lib/ObjCopy/wasm/Writer.cpp
+llvm/lib/ObjCopy/wasm/Writer.h
+llvm/lib/ObjCopy/wasm/Object.h
+llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp
+llvm/lib/ObjCopy/ELF/Object.cpp
+llvm/lib/ObjCopy/MachO/MachOWriter.cpp
+llvm/lib/ObjCopy/MachO/Object.cpp
+llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.h
+llvm/lib/ObjCopy/MachO/MachOWriter.h
+llvm/lib/ObjCopy/MachO/MachOReader.h
+llvm/lib/ObjCopy/MachO/MachOReader.cpp
+llvm/lib/ObjCopy/MachO/Object.h
+llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp
+llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp
+llvm/lib/ObjCopy/COFF/Reader.cpp
+llvm/lib/ObjCopy/COFF/Reader.h
+llvm/lib/ObjCopy/COFF/Object.cpp
+llvm/lib/ObjCopy/COFF/Writer.cpp
+llvm/lib/ObjCopy/COFF/COFFObjcopy.cpp
+llvm/lib/ObjCopy/COFF/Writer.h
+llvm/lib/ObjCopy/COFF/Object.h
 llvm/lib/Object/Archive.cpp
 llvm/lib/Object/Binary.cpp
 llvm/lib/Object/Decompressor.cpp
@@ -6629,42 +6669,9 @@ 
llvm/tools/llvm-microsoft-demangle-fuzzer/DummyDemanglerFuzzer.cpp
 llvm/tools/llvm-microsoft-demangle-fuzzer/llvm-microsoft-demangle-fuzzer.cpp
 llvm/tools/llvm-ml/Disassembler.h
 llvm/tools/llvm-modextract/llvm-modextract.cpp
-llvm/tools/llvm-objcopy/CommonConfig.h
-llvm/tools/llvm-objcopy/ConfigManager.h
 llvm/tools/llvm-objcopy/llvm-objcopy.cpp
-llvm/tools/llvm-objcopy/llvm-objcopy.h
-llvm/tools/llvm-objcopy/MultiFormatConfig.h
-llvm/tools/llvm-objcopy/COFF/COFFConfig.h
-llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp
-llvm/tools/llvm-objcopy/COFF/COFFObjcopy.h
-llvm/tools/llvm-objcopy/COFF/Object.cpp
-llvm/tools/llvm-objcopy/COFF/Object.h
-llvm/tools/llvm-objcopy/COFF/Reader.cpp
-llvm/tools/llvm-objcopy/COFF/Reader.h
-llvm/tools/llvm-objcopy/COFF/Writer.cpp
-llvm/tools/llvm-objcopy/COFF/Writer.h
-llvm/tools/llvm-objcopy/ELF/ELFConfig.h
-llvm/tools/llvm-objcopy/ELF/ELFObjcopy.h
-llvm/tools/llvm-objcopy/MachO/MachOConfig.h
-llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp
-llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.h
-llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp
-llvm/tools/llvm-objcopy/MachO/MachOObjcopy.h
-llvm/tools/llvm-objcopy/MachO/MachOReader.cpp
-llvm/tools/llvm-objcopy/MachO/MachOReader.h
-llvm/tools/llvm-objcopy/MachO/MachOWriter.cpp
-llvm/tools/llvm-objcopy/MachO/MachOWriter.h
-llvm/tools/llvm-objcopy/MachO/Object.cpp
-llvm/tools/llvm-objcopy/MachO/Object.h
-llvm/tools/llvm-objcopy/wasm/Object.cpp
-llvm/tools/llvm-objcopy/wasm/Object.h
-llvm/tools/llvm-objcopy/wasm/Reader.cpp
-llvm/tools/llvm-objcopy/wasm/Reader.h
-llvm/tools/llvm-objcopy/wasm/WasmConfig.h
-llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp
-llvm/tools/llvm-objcopy/wasm/WasmObjcopy.h
-llvm/tools/llvm-objcopy/wasm/Writer.cpp
-llvm/tools/llvm-objcopy/wasm/Writer.h
+llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+llvm/tools/llvm-objcopy/ObjcopyOptions.h
 llvm/tools/llvm-objdump/COFFDump.h
 llvm/tools/llvm-objdump/ELFDump.h
 llvm/tools/llvm-objdump/MachODump.h
@@ -6916,6 +6923,7 @@ llvm/unittests/MC/MCInstPrinter.cpp
 l

[PATCH] D114714: [C++20][Modules][2/8] Add enumerations for partition modules and stream them.

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added a comment.

please remind me when this is committed that I'll need to adjust D118352 
 -- at the moment that's searching for ':' to 
determine partitionness


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114714

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

[PATCH] D120066: FileCheck’s regexp stumped on Windows.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Commit message isn't particularly good either.

Perhaps something like:

> [NFC] Fix debug-info-hotpatch.cpp failure due to downstream regex issue
>
> In our downstream, we discovered that the that the `.*` wildcard 
> in debug-info-hotpatch.cpp (added https://reviews.llvm.org/D116511)
> ended up matching the entire line on our Windows configurations, causing
> the `-function-padmin` check to already be consumed. After digging into it
> we weren't able to find any sort of reason why the platform would matter
> here, however we suspect there must be some difference in the regex matcher
> between systems.
>
> This NFC patch replaces the regex with a more conservative regex that prevents
> this from happening by replacing the `.` match with an 'everything but double-
> quote match, `[^"]`.






Comment at: clang/test/CodeGenCXX/debug-info-hotpatch.cpp:16
+// FUNCTIONPADMIN: {{link[^"]*"}} 
+// FUNCTIONPADMIN: "-functionpadmin"
 

This part of the change shouldn't be necessary, right?  I'd probably prefer not 
to have this part unless needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120066

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