[PATCH] D146408: [clang][Interp] Start supporting complex types

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/PrimType.h:108
+switch (Expr) {
\
+  TYPE_SWITCH_CASE(PT_Sint8, B)
\
+  TYPE_SWITCH_CASE(PT_Uint8, B)
\

aaron.ballman wrote:
> Oh boy, this is going to be interesting, isn't it?
> ```
> consteval _Complex _BitInt(12) UhOh() { return (_Complex _BitInt(12))1; }
> consteval _Complex _BitInt(18) Why() { return (_Complex _BitInt(18))1; }
> 
> static_assert(UhOh() + Why() == 2);
> ```
> 
`_BitInt` isn't supported in the new interpreter at all right now, so this just 
gets rejected.


Apart from that... is a complex bitint really something that should be 
supported? Not just in the new interpreter but generally?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146408

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

[PATCH] D147111: [clang-format] Add MinDigits suboptions to IntegerLiteralSeparator

[Owen Pan via Phabricator via cfe-commits]

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

Closes https://github.com/llvm/llvm-project/issues/61209.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147111

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
  clang/lib/Format/IntegerLiteralSeparatorFixer.h
  clang/unittests/Format/IntegerLiteralSeparatorTest.cpp

Index: clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
===
--- clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
+++ clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
@@ -116,6 +116,41 @@
   verifyFormat("o = 0o43n;", Style);
 }
 
+TEST_F(IntegerLiteralSeparatorTest, MinDigits) {
+  FormatStyle Style = getLLVMStyle();
+  Style.IntegerLiteralSeparator.Binary = 3;
+  Style.IntegerLiteralSeparator.Decimal = 3;
+  Style.IntegerLiteralSeparator.Hex = 2;
+
+  Style.IntegerLiteralSeparator.BinaryMinDigits = 7;
+  verifyFormat("b1 = 0b101101;\n"
+   "b2 = 0b1'101'101;",
+   "b1 = 0b101'101;\n"
+   "b2 = 0b1101101;",
+   Style);
+
+  Style.IntegerLiteralSeparator.DecimalMinDigits = 5;
+  verifyFormat("d1 = 2023;\n"
+   "d2 = 10'000;",
+   "d1 = 2'023;\n"
+   "d2 = 100'00;",
+   Style);
+
+  Style.IntegerLiteralSeparator.DecimalMinDigits = 3;
+  verifyFormat("d1 = 123;\n"
+   "d2 = 1'234;",
+   "d1 = 12'3;\n"
+   "d2 = 12'34;",
+   Style);
+
+  Style.IntegerLiteralSeparator.HexMinDigits = 6;
+  verifyFormat("h1 = 0xABCDE;\n"
+   "h2 = 0xAB'CD'EF;",
+   "h1 = 0xA'BC'DE;\n"
+   "h2 = 0xABC'DEF;",
+   Style);
+}
+
 TEST_F(IntegerLiteralSeparatorTest, FixRanges) {
   FormatStyle Style = getLLVMStyle();
   Style.IntegerLiteralSeparator.Decimal = 3;
Index: clang/lib/Format/IntegerLiteralSeparatorFixer.h
===
--- clang/lib/Format/IntegerLiteralSeparatorFixer.h
+++ clang/lib/Format/IntegerLiteralSeparatorFixer.h
@@ -27,7 +27,8 @@
 
 private:
   bool checkSeparator(const StringRef IntegerLiteral, int DigitsPerGroup) const;
-  std::string format(const StringRef IntegerLiteral, int DigitsPerGroup) const;
+  std::string format(const StringRef IntegerLiteral, int DigitsPerGroup,
+ int DigitCount, bool RemoveSeparator) const;
 
   char Separator;
 };
Index: clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
===
--- clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
+++ clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
@@ -69,6 +69,12 @@
   if (SkipBinary && SkipDecimal && SkipHex)
 return {};
 
+  const auto BinaryMinDigits =
+  std::max((int)Option.BinaryMinDigits, Binary + 1);
+  const auto DecimalMinDigits =
+  std::max((int)Option.DecimalMinDigits, Decimal + 1);
+  const auto HexMinDigits = std::max((int)Option.HexMinDigits, Hex + 1);
+
   const auto  = Env.getSourceManager();
   AffectedRangeManager AffectedRangeMgr(SourceMgr, Env.getCharRanges());
 
@@ -116,11 +122,6 @@
 (IsBase16 && Text.find_last_of(".pP") != StringRef::npos)) {
   continue;
 }
-if (((IsBase2 && Binary < 0) || (IsBase10 && Decimal < 0) ||
- (IsBase16 && Hex < 0)) &&
-Text.find(Separator) == StringRef::npos) {
-  continue;
-}
 const auto Start = Text[0] == '0' ? 2 : 0;
 auto End = Text.find_first_of("uUlLzZn", Start);
 if (End == StringRef::npos)
@@ -130,13 +131,25 @@
   Text = Text.substr(Start, Length);
 }
 auto DigitsPerGroup = Decimal;
-if (IsBase2)
+auto MinDigits = DecimalMinDigits;
+if (IsBase2) {
   DigitsPerGroup = Binary;
-else if (IsBase16)
+  MinDigits = BinaryMinDigits;
+} else if (IsBase16) {
   DigitsPerGroup = Hex;
-if (DigitsPerGroup > 0 && checkSeparator(Text, DigitsPerGroup))
+  MinDigits = HexMinDigits;
+}
+const auto SeparatorCount = Text.count(Separator);
+const int DigitCount = Length - SeparatorCount;
+const bool RemoveSeparator = DigitsPerGroup < 0 || DigitCount < MinDigits;
+if (RemoveSeparator && SeparatorCount == 0)
   continue;
-const auto  = format(Text, DigitsPerGroup);
+if (!RemoveSeparator && SeparatorCount > 0 &&
+checkSeparator(Text, DigitsPerGroup)) {
+  continue;
+}
+const auto  =
+format(Text, DigitsPerGroup, DigitCount, RemoveSeparator);
 assert(Formatted != Text);
 if (Start > 0)
   Location = 

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Lex/ModuleMap.cpp:935
+  // with any legal user-defined module name).
+  StringRef IName = ".ImplementationUnit";
+  assert(!Modules[IName] && "multiple implementation units?");

iains wrote:
> ChuanqiXu wrote:
> > nit: It should more consistent to use ``.
> (I do not mind making this change later, if you like - but I did not want to 
> repeat the test cycle today).
> 
> In this case I think that `<>` would not mean the same thing as it does in 
> `` and `` modules fragments,  These are never entered into 
> the module map - but are always owned by a parent module.
> 
> In the case of the ImplementationUnit - the *name* of the module is unchanged 
> (i.e. it would still be called `M`  when the module line is ` module M;`.
> 
> The `.ImplementationUnit` is not the name of the module - but rather it is a 
> key into the module map (which needs to be different from the name of the 
> interface),  Since there can only be one Implementation Unit in a given 
> session, it is safe to use a fixed key. 
> 
> However, I do not mind changing the key to `` if you 
> think that it would be more clear.
> 
Thanks, I think it doesn't matter too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126959

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

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

[Iain Sandoe via Phabricator via cfe-commits]

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



Comment at: clang/lib/Lex/ModuleMap.cpp:935
+  // with any legal user-defined module name).
+  StringRef IName = ".ImplementationUnit";
+  assert(!Modules[IName] && "multiple implementation units?");

ChuanqiXu wrote:
> nit: It should more consistent to use ``.
(I do not mind making this change later, if you like - but I did not want to 
repeat the test cycle today).

In this case I think that `<>` would not mean the same thing as it does in 
`` and `` modules fragments,  These are never entered into the 
module map - but are always owned by a parent module.

In the case of the ImplementationUnit - the *name* of the module is unchanged 
(i.e. it would still be called `M`  when the module line is ` module M;`.

The `.ImplementationUnit` is not the name of the module - but rather it is a 
key into the module map (which needs to be different from the name of the 
interface),  Since there can only be one Implementation Unit in a given 
session, it is safe to use a fixed key. 

However, I do not mind changing the key to `` if you think 
that it would be more clear.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126959

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:980-981
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)

nickdesaulniers wrote:
> The following input causes clang to crash:
> 
> ```
> $ clang -fno-integrated-as -c -x assembler-with-cpp /dev/null
> clang: /android0/llvm-project/llvm/include/llvm/ADT/StringRef.h:597: 
> llvm::StringRef llvm::StringRef::drop_front(size_t) const: Assertion `size() 
> >= N && "Dropping more elements than exist"' failed.
> ```
> 
> Please fix that and add a test case for `/dev/null` (i.e. inputs for which no 
> extension exists).
For Inputs without extension, will there always be a -x flag?


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

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added a comment.

In D145726#4228406 , @MaskRay wrote:

> Please update the commit message and comment about what binutils versions 
> reject the construct.

I already added this - "Due to this we started seeing
below assembler error with GNU binutils version 2.34 and below. This
error is not shown by binutils version 2.36 and later."


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

https://reviews.llvm.org/D145726

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

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

[Iain Sandoe via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6e4f870a21e3: re-land [C++20][Modules] Introduce an 
implementation module. (authored by iains).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126959

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Lex/ModuleMap.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CXX/module/basic/basic.def.odr/p4.cppm
  clang/test/CXX/module/basic/basic.link/p2.cppm
  clang/test/CodeGenCXX/module-intializer.cpp

Index: clang/test/CodeGenCXX/module-intializer.cpp
===
--- clang/test/CodeGenCXX/module-intializer.cpp
+++ clang/test/CodeGenCXX/module-intializer.cpp
@@ -18,17 +18,17 @@
 // RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
 
 // RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
-// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN: -fmodule-file=N=N.pcm -fmodule-file=O=O.pcm -fmodule-file=M:Part=M-part.pcm \
 // RUN:-emit-module-interface -o M.pcm
 // RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
 // RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
 
 // RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
-// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: -fmodule-file=M=M.pcm -S -emit-llvm  -o - \
 // RUN: | FileCheck %s --check-prefix=CHECK-USE
 
 // RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-impl.cpp \
-// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: -fmodule-file=M=M.pcm -S -emit-llvm  -o - \
 // RUN: | FileCheck %s --check-prefix=CHECK-IMPL
 
 // RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp -S -emit-llvm \
@@ -41,7 +41,7 @@
 // RUN:   -o - | FileCheck %s --check-prefix=CHECK-P
 
 // RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
-// RUN:   -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:   -fmodule-file=N.pcm -fmodule-file=O=O.pcm -fmodule-file=M:Part=M-part.pcm \
 // RUN:   -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-M
 
 //--- N-h.h
Index: clang/test/CXX/module/basic/basic.link/p2.cppm
===
--- clang/test/CXX/module/basic/basic.link/p2.cppm
+++ clang/test/CXX/module/basic/basic.link/p2.cppm
@@ -39,19 +39,21 @@
 }
 
 //--- M.cpp
-// expected-no-diagnostics
+
 module M;
 
-// FIXME: Use of internal linkage entities should be rejected.
 void use_from_module_impl() {
   external_linkage_fn();
   module_linkage_fn();
-  internal_linkage_fn();
+  internal_linkage_fn(); // expected-error {{no matching function for call to 'internal_linkage_fn'}}
   (void)external_linkage_class{};
   (void)module_linkage_class{};
-  (void)internal_linkage_class{};
   (void)external_linkage_var;
   (void)module_linkage_var;
+
+  // FIXME: Issue #61427 Internal-linkage declarations in the interface TU
+  // should not be not visible here.
+  (void)internal_linkage_class{};
   (void)internal_linkage_var;
 }
 
Index: clang/test/CXX/module/basic/basic.def.odr/p4.cppm
===
--- clang/test/CXX/module/basic/basic.def.odr/p4.cppm
+++ clang/test/CXX/module/basic/basic.def.odr/p4.cppm
@@ -143,9 +143,6 @@
   (void)_var_exported;
   (void)_var_exported;
 
-  // CHECK: define {{.*}}@_ZL26used_static_module_linkagev
-  used_static_module_linkage();
-
   // CHECK: define linkonce_odr {{.*}}@_ZW6Module26used_inline_module_linkagev
   used_inline_module_linkage();
 
@@ -154,8 +151,12 @@
 
   (void)_var_module_linkage;
   (void)_var_module_linkage;
+
+  // FIXME: Issue #61427 Internal-linkage declarations in the interface TU
+  // should not be not visible here.
   (void)_var_module_linkage; // FIXME: Should not be visible here.
-  (void)_var_module_linkage;
+
+  (void)_var_module_linkage; // FIXME: will be visible after P2788R0
 }
 
 //--- user.cpp
@@ -176,5 +177,6 @@
   (void)_var_exported;
   (void)_var_exported;
 
+  // Internal-linkage declarations are not visible here.
   // Module-linkage declarations are not visible here.
 }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2719,7 +2719,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, 3)); // Kind
+  

[clang] 6e4f870 - re-land [C++20][Modules] Introduce an implementation module.

[Iain Sandoe via cfe-commits]

Author: Iain Sandoe
Date: 2023-03-29T08:52:28+05:30
New Revision: 6e4f870a21e344fdcd61fe613b0aeeafb8a84ed2

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

LOG: re-land [C++20][Modules] Introduce an implementation module.

We need to be able to distinguish individual TUs from the same module in cases
where TU-local entities either need to be hidden (or, for some cases of ADL in
template instantiation, need to be detected as exposures).

This creates a module type for the implementation which implicitly imports its
primary module interface per C++20:
[module.unit/8] 'A module-declaration that contains neither an export-keyword
nor a module-partition implicitly imports the primary module interface unit of
the module as if by a module-import-declaration.

Implementation modules are never serialized (-emit-module-interface for an
implementation unit is diagnosed and rejected).

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

Added: 


Modified: 
clang/include/clang/Basic/Module.h
clang/include/clang/Lex/ModuleMap.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/Decl.cpp
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaModule.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/CXX/module/basic/basic.def.odr/p4.cppm
clang/test/CXX/module/basic/basic.link/p2.cppm
clang/test/CodeGenCXX/module-intializer.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/Module.h 
b/clang/include/clang/Basic/Module.h
index 387ce4d6e9b17..c0c99eb8b6d62 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -103,16 +103,22 @@ class alignas(8) Module {
   /// The location of the module definition.
   SourceLocation DefinitionLoc;
 
+  // FIXME: Consider if reducing the size of this enum (having Partition and
+  // Named modules only) then representing interface/implementation separately
+  // is more efficient.
   enum ModuleKind {
 /// This is a module that was defined by a module map and built out
 /// of header files.
 ModuleMapModule,
 
+/// This is a C++ 20 header unit.
+ModuleHeaderUnit,
+
 /// This is a C++20 module interface unit.
 ModuleInterfaceUnit,
 
-/// This is a C++ 20 header unit.
-ModuleHeaderUnit,
+/// This is a C++20 module implementation unit.
+ModuleImplementationUnit,
 
 /// This is a C++ 20 module partition interface.
 ModulePartitionInterface,
@@ -169,9 +175,16 @@ class alignas(8) Module {
   /// Does this Module scope describe part of the purview of a standard named
   /// C++ module?
   bool isModulePurview() const {
-return Kind == ModuleInterfaceUnit || Kind == ModulePartitionInterface ||
-   Kind == ModulePartitionImplementation ||
-   Kind == PrivateModuleFragment;
+switch (Kind) {
+case ModuleInterfaceUnit:
+case ModuleImplementationUnit:
+case ModulePartitionInterface:
+case ModulePartitionImplementation:
+case PrivateModuleFragment:
+  return true;
+default:
+  return false;
+}
   }
 
   /// Does this Module scope describe a fragment of the global module within
@@ -561,6 +574,11 @@ class alignas(8) Module {
Kind == ModulePartitionImplementation;
   }
 
+  /// Is this a module implementation.
+  bool isModuleImplementation() const {
+return Kind == ModuleImplementationUnit;
+  }
+
   /// Is this module a header unit.
   bool isHeaderUnit() const { return Kind == ModuleHeaderUnit; }
   // Is this a C++20 module interface or a partition.

diff  --git a/clang/include/clang/Lex/ModuleMap.h 
b/clang/include/clang/Lex/ModuleMap.h
index a0ddd13c11bfd..f155c609b06cb 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -560,6 +560,11 @@ class ModuleMap {
   Module *createPrivateModuleFragmentForInterfaceUnit(Module *Parent,
   SourceLocation Loc);
 
+  /// Create a new C++ module with the specified kind, and reparent any pending
+  /// global module fragment(s) to it.
+  Module *createModuleUnitWithKind(SourceLocation Loc, StringRef Name,
+   Module::ModuleKind Kind);
+
   /// Create a new module for a C++ module interface unit.
   /// The module must not already exist, and will be configured for the current
   /// compilation.
@@ -569,6 +574,13 @@ class ModuleMap {
   /// \returns The newly-created module.
   Module *createModuleForInterfaceUnit(SourceLocation Loc, StringRef Name);
 
+  /// Create a new module for a C++ module implementation unit.
+  /// The interface module for this implementation 

[clang] 279c7a2 - Revert "[C++20] [Modules] Don't load declaration eagerly for named modules"

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-03-29T11:15:38+08:00
New Revision: 279c7a2f17937836ed13e359c3fb381bef7defaf

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

LOG: Revert "[C++20] [Modules] Don't load declaration eagerly for named modules"

This reverts commit af86957cbbffd3dfff3c6750ebddf118aebd0069.

Close https://github.com/llvm/llvm-project/issues/61733.

Previously I banned the eagerly loading for declarations from named
modules to speedup the process of reading modules. But I didn't think
about special decls like PragmaCommentDecl and PragmaDetectMismatchDecl.
So here is the issue https://github.com/llvm/llvm-project/issues/61733.

Note that the current behavior is still incorrect. Given:

```
// mod.cppm
module;

export module mod;
```

and

```
// user.cpp
import mod;
```

Now the IR of `user.cpp` will contain the metadata '!0 =
!{!"msvcprt.lib"}' incorrectly. The root cause of the problem is that
`EagerlyDeserializedDecls` is designed for headers and it didn't take
care for named modules. We need to redesign a new mechanism for named
modules.

Added: 


Modified: 
clang/lib/Serialization/ASTWriterDecl.cpp

Removed: 
clang/test/Modules/no-eager-load.cppm



diff  --git a/clang/lib/Serialization/ASTWriterDecl.cpp 
b/clang/lib/Serialization/ASTWriterDecl.cpp
index bbd3c36df2f96..69d192612bccf 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2468,13 +2468,7 @@ void ASTWriter::WriteDeclAbbrevs() {
 /// relatively painless since they would presumably only do it for top-level
 /// decls.
 static bool isRequiredDecl(const Decl *D, ASTContext ,
-   Module *WritingModule) {
-  // Named modules have 
diff erent semantics than header modules. Every named
-  // module units owns a translation unit. So the importer of named modules
-  // doesn't need to deserilize everything ahead of time.
-  if (WritingModule && WritingModule->isModulePurview())
-return false;
-
+   bool WritingModule) {
   // An ObjCMethodDecl is never considered as "required" because its
   // implementation container always is.
 

diff  --git a/clang/test/Modules/no-eager-load.cppm 
b/clang/test/Modules/no-eager-load.cppm
deleted file mode 100644
index 6632cc60c8eb8..0
--- a/clang/test/Modules/no-eager-load.cppm
+++ /dev/null
@@ -1,110 +0,0 @@
-// RUN: rm -rf %t
-// RUN: split-file %s %t
-// RUN: cd %t
-//
-// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/a.cppm -o %t/a.pcm
-// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/b.cppm -o %t/b.pcm
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/c.cpp \
-// RUN: -fprebuilt-module-path=%t
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/d.cpp \
-// RUN: -fprebuilt-module-path=%t
-//
-// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/e.cppm -o %t/e.pcm
-// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/f.cppm -o %t/f.pcm
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/g.cpp \
-// RUN: -fprebuilt-module-path=%t
-//
-// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/h.cppm \
-// RUN: -fprebuilt-module-path=%t -o %t/h.pcm
-// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/i.cppm \
-// RUN: -fprebuilt-module-path=%t -o %t/i.pcm
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/j.cpp \
-// RUN: -fprebuilt-module-path=%t
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/k.cpp \
-// RUN: -fprebuilt-module-path=%t
-
-//--- a.cppm
-export module a;
-export void foo() {
-
-}
-
-//--- b.cppm
-export module b;
-void bar();
-export void foo() {
-bar();
-}
-
-//--- c.cpp
-// expected-no-diagnostics
-// Since we will load all the declaration lazily, we won't be able to find
-// the ODR violation here.
-import a;
-import b;
-
-//--- d.cpp
-import a;
-import b;
-// Test that we can still check the odr violation if we call the function
-// actually.
-void use() {
-foo(); // expected-error@* {{'foo' has 
diff erent definitions in 
diff erent modules;}}
-   // expected-note@* {{but in 'a' found a 
diff erent body}}
-}
-
-//--- foo.h
-void foo() {
-
-}
-
-//--- bar.h
-void bar();
-void foo() {
-bar();
-}
-
-//--- e.cppm
-module;
-#include "foo.h"
-export module e;
-export using ::foo;
-
-//--- f.cppm
-module;
-#include "bar.h"
-export module f;
-export using ::foo;
-
-//--- g.cpp
-import e;
-import f;
-void use() {
-foo(); // expected-error@* {{'foo' has 
diff erent definitions in 
diff erent modules;}}
-   // expected-note@* {{but in 'e.' found a 
diff erent body}}
-}
-
-//--- h.cppm
-export module h;
-export import a;
-export import b;
-
-//--- i.cppm
-export module i;
-export import e;
-export import f;
-
-//--- j.cpp
-import h;
-void use() 

[PATCH] D146926: [clang-format] Add option to decorate reflowed block comments

[Adheesh Wadkar via Phabricator via cfe-commits]

apwadkar added inline comments.



Comment at: clang/lib/Format/BreakableToken.cpp:406
   Decoration = "* ";
-  if (Lines.size() == 1 && !FirstInLine) {
+  if ((Lines.size() == 1 && !FirstInLine) || !Style.DecorateReflowedComments) {
 // Comments for which FirstInLine is false can start on arbitrary column,

HazardyKnusperkeks wrote:
> So if set to `false` it will never add the decoration. But when set to `true` 
> it still will not, if it's the first line.
> 
> This is not what I'd expect reading the documentation.
> 
> If you want to pursue this I think you should go for an enum with `Never`, 
> `Always`, and `` (what we currently have).
> So if set to `false` it will never add the decoration. But when set to `true` 
> it still will not, if it's the first line.
> 
> This is not what I'd expect reading the documentation.

Yeah, I think that makes sense. That's what I was thinking while working on the 
unit tests, because I realized this was breaking the existing style.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146926

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

[clang] 766d048 - [clang-repl] Use std::move when converting Error to Expected

[Anubhab Ghosh via cfe-commits]

Author: Anubhab Ghosh
Date: 2023-03-29T08:18:36+05:30
New Revision: 766d048d819a78443da73f67afa04e0a108412b6

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

LOG: [clang-repl] Use std::move when converting Error to Expected

Added: 


Modified: 
clang/lib/Interpreter/Interpreter.cpp

Removed: 




diff  --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 76d5f162a34a5..a0ccbc20b95f4 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -207,7 +207,7 @@ const CompilerInstance *Interpreter::getCompilerInstance() 
const {
 llvm::Expected Interpreter::getExecutionEngine() {
   if (!IncrExecutor) {
 if (auto Err = CreateExecutor())
-  return Err;
+  return std::move(Err);
   }
 
   return IncrExecutor->GetExecutionEngine();



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

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

[Anubhab Ghosh via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd978730d8e2c: [clang-repl] Add a command to load dynamic 
libraries (authored by argentite).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141824

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/test/Interpreter/Inputs/dynamic-library-test.cpp
  clang/test/Interpreter/dynamic-library.cpp
  clang/tools/clang-repl/ClangRepl.cpp

Index: clang/tools/clang-repl/ClangRepl.cpp
===
--- clang/tools/clang-repl/ClangRepl.cpp
+++ clang/tools/clang-repl/ClangRepl.cpp
@@ -123,6 +123,13 @@
 }
 continue;
   }
+  if (Line->rfind("%lib ", 0) == 0) {
+if (auto Err = Interp->LoadDynamicLibrary(Line->data() + 5)) {
+  llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
+  HasError = true;
+}
+continue;
+  }
 
   if (auto Err = Interp->ParseAndExecute(*Line)) {
 llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
Index: clang/test/Interpreter/dynamic-library.cpp
===
--- /dev/null
+++ clang/test/Interpreter/dynamic-library.cpp
@@ -0,0 +1,19 @@
+// REQUIRES: host-supports-jit, system-linux
+
+// RUN: %clang -xc++ -o %T/libdynamic-library-test.so -fPIC -shared -DLIBRARY %S/Inputs/dynamic-library-test.cpp
+// RUN: cat %s | env LD_LIBRARY_PATH=%T:$LD_LIBRARY_PATH clang-repl | FileCheck %s
+
+#include 
+
+extern int ultimate_answer;
+int calculate_answer();
+
+%lib libdynamic-library-test.so
+
+printf("Return value: %d\n", calculate_answer());
+// CHECK: Return value: 5
+
+printf("Variable: %d\n", ultimate_answer);
+// CHECK-NEXT: Variable: 42
+
+%quit
Index: clang/test/Interpreter/Inputs/dynamic-library-test.cpp
===
--- /dev/null
+++ clang/test/Interpreter/Inputs/dynamic-library-test.cpp
@@ -0,0 +1,6 @@
+int ultimate_answer = 0;
+
+int calculate_answer() {
+  ultimate_answer = 42;
+  return 5;
+}
Index: clang/lib/Interpreter/Interpreter.cpp
===
--- clang/lib/Interpreter/Interpreter.cpp
+++ clang/lib/Interpreter/Interpreter.cpp
@@ -29,6 +29,7 @@
 #include "clang/Frontend/TextDiagnosticBuffer.h"
 #include "clang/Lex/PreprocessorOptions.h"
 
+#include "llvm/ExecutionEngine/Orc/LLJIT.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/TargetParser/Host.h"
@@ -203,10 +204,13 @@
   return IncrParser->getCI();
 }
 
-const llvm::orc::LLJIT *Interpreter::getExecutionEngine() const {
-  if (IncrExecutor)
-return IncrExecutor->getExecutionEngine();
-  return nullptr;
+llvm::Expected Interpreter::getExecutionEngine() {
+  if (!IncrExecutor) {
+if (auto Err = CreateExecutor())
+  return Err;
+  }
+
+  return IncrExecutor->GetExecutionEngine();
 }
 
 llvm::Expected
@@ -214,14 +218,21 @@
   return IncrParser->Parse(Code);
 }
 
+llvm::Error Interpreter::CreateExecutor() {
+  const clang::TargetInfo  =
+  getCompilerInstance()->getASTContext().getTargetInfo();
+  llvm::Error Err = llvm::Error::success();
+  auto Executor = std::make_unique(*TSCtx, Err, TI);
+  if (!Err)
+IncrExecutor = std::move(Executor);
+
+  return Err;
+}
+
 llvm::Error Interpreter::Execute(PartialTranslationUnit ) {
   assert(T.TheModule);
   if (!IncrExecutor) {
-const clang::TargetInfo  =
-getCompilerInstance()->getASTContext().getTargetInfo();
-llvm::Error Err = llvm::Error::success();
-IncrExecutor = std::make_unique(*TSCtx, Err, TI);
-
+auto Err = CreateExecutor();
 if (Err)
   return Err;
   }
@@ -283,3 +294,19 @@
   }
   return llvm::Error::success();
 }
+
+llvm::Error Interpreter::LoadDynamicLibrary(const char *name) {
+  auto EE = getExecutionEngine();
+  if (!EE)
+return EE.takeError();
+
+  auto  = EE->getDataLayout();
+
+  if (auto DLSG = llvm::orc::DynamicLibrarySearchGenerator::Load(
+  name, DL.getGlobalPrefix()))
+EE->getMainJITDylib().addGenerator(std::move(*DLSG));
+  else
+return DLSG.takeError();
+
+  return llvm::Error::success();
+}
Index: clang/lib/Interpreter/IncrementalExecutor.h
===
--- clang/lib/Interpreter/IncrementalExecutor.h
+++ clang/lib/Interpreter/IncrementalExecutor.h
@@ -53,7 +53,8 @@
   llvm::Error cleanUp();
   llvm::Expected
   getSymbolAddress(llvm::StringRef Name, SymbolNameKind NameKind) const;
-  llvm::orc::LLJIT *getExecutionEngine() const { return Jit.get(); }
+
+  llvm::orc::LLJIT () { return *Jit; }
 };
 
 } // end namespace clang
Index: clang/include/clang/Interpreter/Interpreter.h

[clang] d978730 - [clang-repl] Add a command to load dynamic libraries

[Anubhab Ghosh via cfe-commits]

Author: Anubhab Ghosh
Date: 2023-03-29T08:04:50+05:30
New Revision: d978730d8e2c10c76867b83bec2f1143d895ee7d

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

LOG: [clang-repl] Add a command to load dynamic libraries

This commit adds the %lib  command to load a dynamic library to be
used by the currently running interpreted code.
For example `%lib libSDL2.so`.

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

Added: 
clang/test/Interpreter/Inputs/dynamic-library-test.cpp
clang/test/Interpreter/dynamic-library.cpp

Modified: 
clang/include/clang/Interpreter/Interpreter.h
clang/lib/Interpreter/IncrementalExecutor.h
clang/lib/Interpreter/Interpreter.cpp
clang/tools/clang-repl/ClangRepl.cpp

Removed: 




diff  --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index fd22af976613..b20d77e8ef85 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -28,7 +28,7 @@ namespace llvm {
 namespace orc {
 class LLJIT;
 class ThreadSafeContext;
-}
+} // namespace orc
 } // namespace llvm
 
 namespace clang {
@@ -52,12 +52,15 @@ class Interpreter {
 
   Interpreter(std::unique_ptr CI, llvm::Error );
 
+  llvm::Error CreateExecutor();
+
 public:
   ~Interpreter();
   static llvm::Expected>
   create(std::unique_ptr CI);
   const CompilerInstance *getCompilerInstance() const;
-  const llvm::orc::LLJIT *getExecutionEngine() const;
+  llvm::Expected getExecutionEngine();
+
   llvm::Expected Parse(llvm::StringRef Code);
   llvm::Error Execute(PartialTranslationUnit );
   llvm::Error ParseAndExecute(llvm::StringRef Code) {
@@ -72,6 +75,9 @@ class Interpreter {
   /// Undo N previous incremental inputs.
   llvm::Error Undo(unsigned N = 1);
 
+  /// Link a dynamic library
+  llvm::Error LoadDynamicLibrary(const char *name);
+
   /// \returns the \c JITTargetAddress of a \c GlobalDecl. This interface uses
   /// the CodeGenModule's internal mangling cache to avoid recomputing the
   /// mangled name.

diff  --git a/clang/lib/Interpreter/IncrementalExecutor.h 
b/clang/lib/Interpreter/IncrementalExecutor.h
index 54d37c76326b..f7922ecb5380 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.h
+++ b/clang/lib/Interpreter/IncrementalExecutor.h
@@ -53,7 +53,8 @@ class IncrementalExecutor {
   llvm::Error cleanUp();
   llvm::Expected
   getSymbolAddress(llvm::StringRef Name, SymbolNameKind NameKind) const;
-  llvm::orc::LLJIT *getExecutionEngine() const { return Jit.get(); }
+
+  llvm::orc::LLJIT () { return *Jit; }
 };
 
 } // end namespace clang

diff  --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 3f0842c567da..76d5f162a34a 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -29,6 +29,7 @@
 #include "clang/Frontend/TextDiagnosticBuffer.h"
 #include "clang/Lex/PreprocessorOptions.h"
 
+#include "llvm/ExecutionEngine/Orc/LLJIT.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/TargetParser/Host.h"
@@ -203,10 +204,13 @@ const CompilerInstance 
*Interpreter::getCompilerInstance() const {
   return IncrParser->getCI();
 }
 
-const llvm::orc::LLJIT *Interpreter::getExecutionEngine() const {
-  if (IncrExecutor)
-return IncrExecutor->getExecutionEngine();
-  return nullptr;
+llvm::Expected Interpreter::getExecutionEngine() {
+  if (!IncrExecutor) {
+if (auto Err = CreateExecutor())
+  return Err;
+  }
+
+  return IncrExecutor->GetExecutionEngine();
 }
 
 llvm::Expected
@@ -214,14 +218,21 @@ Interpreter::Parse(llvm::StringRef Code) {
   return IncrParser->Parse(Code);
 }
 
+llvm::Error Interpreter::CreateExecutor() {
+  const clang::TargetInfo  =
+  getCompilerInstance()->getASTContext().getTargetInfo();
+  llvm::Error Err = llvm::Error::success();
+  auto Executor = std::make_unique(*TSCtx, Err, TI);
+  if (!Err)
+IncrExecutor = std::move(Executor);
+
+  return Err;
+}
+
 llvm::Error Interpreter::Execute(PartialTranslationUnit ) {
   assert(T.TheModule);
   if (!IncrExecutor) {
-const clang::TargetInfo  =
-getCompilerInstance()->getASTContext().getTargetInfo();
-llvm::Error Err = llvm::Error::success();
-IncrExecutor = std::make_unique(*TSCtx, Err, TI);
-
+auto Err = CreateExecutor();
 if (Err)
   return Err;
   }
@@ -283,3 +294,19 @@ llvm::Error Interpreter::Undo(unsigned N) {
   }
   return llvm::Error::success();
 }
+
+llvm::Error Interpreter::LoadDynamicLibrary(const char *name) {
+  auto EE = getExecutionEngine();
+  if (!EE)
+return EE.takeError();
+
+  auto  = EE->getDataLayout();
+
+  if (auto DLSG = llvm::orc::DynamicLibrarySearchGenerator::Load(
+  name, DL.getGlobalPrefix()))
+

[PATCH] D147030: [Clang][Driver] Default Generic_GCC::IsIntegratedAssemblerDefault to true

[Brad Smith 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 rGd44371c00d87: [Clang][Driver] Default 
Generic_GCC::IsIntegratedAssemblerDefault to true (authored by brad).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147030

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/integrated-as.c


Index: clang/test/Driver/integrated-as.c
===
--- clang/test/Driver/integrated-as.c
+++ clang/test/Driver/integrated-as.c
@@ -12,5 +12,4 @@
 
 // NOFIAS-NOT: cc1as
 // NOFIAS: -cc1
-// NOFIAS: "-fno-verbose-asm"
 // NOFIAS: -no-integrated-as
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2916,44 +2916,12 @@
 
 bool Generic_GCC::IsIntegratedAssemblerDefault() const {
   switch (getTriple().getArch()) {
-  case llvm::Triple::aarch64:
-  case llvm::Triple::aarch64_be:
-  case llvm::Triple::amdgcn:
-  case llvm::Triple::arm:
-  case llvm::Triple::armeb:
-  case llvm::Triple::avr:
-  case llvm::Triple::bpfel:
-  case llvm::Triple::bpfeb:
-  case llvm::Triple::csky:
-  case llvm::Triple::hexagon:
-  case llvm::Triple::lanai:
-  case llvm::Triple::loongarch32:
-  case llvm::Triple::loongarch64:
-  case llvm::Triple::m68k:
-  case llvm::Triple::mips:
-  case llvm::Triple::mipsel:
-  case llvm::Triple::mips64:
-  case llvm::Triple::mips64el:
-  case llvm::Triple::msp430:
-  case llvm::Triple::ppc:
-  case llvm::Triple::ppcle:
-  case llvm::Triple::ppc64:
-  case llvm::Triple::ppc64le:
-  case llvm::Triple::r600:
-  case llvm::Triple::riscv32:
-  case llvm::Triple::riscv64:
-  case llvm::Triple::sparc:
-  case llvm::Triple::sparcel:
-  case llvm::Triple::sparcv9:
-  case llvm::Triple::systemz:
-  case llvm::Triple::thumb:
-  case llvm::Triple::thumbeb:
-  case llvm::Triple::ve:
-  case llvm::Triple::x86:
-  case llvm::Triple::x86_64:
-return true;
-  default:
+  case llvm::Triple::nvptx:
+  case llvm::Triple::nvptx64:
+  case llvm::Triple::xcore:
 return false;
+  default:
+return getTriple().getVendor() != llvm::Triple::Myriad;
   }
 }
 


Index: clang/test/Driver/integrated-as.c
===
--- clang/test/Driver/integrated-as.c
+++ clang/test/Driver/integrated-as.c
@@ -12,5 +12,4 @@
 
 // NOFIAS-NOT: cc1as
 // NOFIAS: -cc1
-// NOFIAS: "-fno-verbose-asm"
 // NOFIAS: -no-integrated-as
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2916,44 +2916,12 @@
 
 bool Generic_GCC::IsIntegratedAssemblerDefault() const {
   switch (getTriple().getArch()) {
-  case llvm::Triple::aarch64:
-  case llvm::Triple::aarch64_be:
-  case llvm::Triple::amdgcn:
-  case llvm::Triple::arm:
-  case llvm::Triple::armeb:
-  case llvm::Triple::avr:
-  case llvm::Triple::bpfel:
-  case llvm::Triple::bpfeb:
-  case llvm::Triple::csky:
-  case llvm::Triple::hexagon:
-  case llvm::Triple::lanai:
-  case llvm::Triple::loongarch32:
-  case llvm::Triple::loongarch64:
-  case llvm::Triple::m68k:
-  case llvm::Triple::mips:
-  case llvm::Triple::mipsel:
-  case llvm::Triple::mips64:
-  case llvm::Triple::mips64el:
-  case llvm::Triple::msp430:
-  case llvm::Triple::ppc:
-  case llvm::Triple::ppcle:
-  case llvm::Triple::ppc64:
-  case llvm::Triple::ppc64le:
-  case llvm::Triple::r600:
-  case llvm::Triple::riscv32:
-  case llvm::Triple::riscv64:
-  case llvm::Triple::sparc:
-  case llvm::Triple::sparcel:
-  case llvm::Triple::sparcv9:
-  case llvm::Triple::systemz:
-  case llvm::Triple::thumb:
-  case llvm::Triple::thumbeb:
-  case llvm::Triple::ve:
-  case llvm::Triple::x86:
-  case llvm::Triple::x86_64:
-return true;
-  default:
+  case llvm::Triple::nvptx:
+  case llvm::Triple::nvptx64:
+  case llvm::Triple::xcore:
 return false;
+  default:
+return getTriple().getVendor() != llvm::Triple::Myriad;
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] d44371c - [Clang][Driver] Default Generic_GCC::IsIntegratedAssemblerDefault to true

[Brad Smith via cfe-commits]

Author: Brad Smith
Date: 2023-03-28T22:26:18-04:00
New Revision: d44371c00d87f73aba4ba0feafb4a18151d6f831

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

LOG: [Clang][Driver] Default Generic_GCC::IsIntegratedAssemblerDefault to true

Invert the logic and have the default being true. Disable the few spots where
it looks like IAS is currently not used.

Reviewed By: MaskRay

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
clang/test/Driver/integrated-as.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 0c8868109f7e..b0716322bc14 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2916,44 +2916,12 @@ bool Generic_GCC::isPICDefaultForced() const {
 
 bool Generic_GCC::IsIntegratedAssemblerDefault() const {
   switch (getTriple().getArch()) {
-  case llvm::Triple::aarch64:
-  case llvm::Triple::aarch64_be:
-  case llvm::Triple::amdgcn:
-  case llvm::Triple::arm:
-  case llvm::Triple::armeb:
-  case llvm::Triple::avr:
-  case llvm::Triple::bpfel:
-  case llvm::Triple::bpfeb:
-  case llvm::Triple::csky:
-  case llvm::Triple::hexagon:
-  case llvm::Triple::lanai:
-  case llvm::Triple::loongarch32:
-  case llvm::Triple::loongarch64:
-  case llvm::Triple::m68k:
-  case llvm::Triple::mips:
-  case llvm::Triple::mipsel:
-  case llvm::Triple::mips64:
-  case llvm::Triple::mips64el:
-  case llvm::Triple::msp430:
-  case llvm::Triple::ppc:
-  case llvm::Triple::ppcle:
-  case llvm::Triple::ppc64:
-  case llvm::Triple::ppc64le:
-  case llvm::Triple::r600:
-  case llvm::Triple::riscv32:
-  case llvm::Triple::riscv64:
-  case llvm::Triple::sparc:
-  case llvm::Triple::sparcel:
-  case llvm::Triple::sparcv9:
-  case llvm::Triple::systemz:
-  case llvm::Triple::thumb:
-  case llvm::Triple::thumbeb:
-  case llvm::Triple::ve:
-  case llvm::Triple::x86:
-  case llvm::Triple::x86_64:
-return true;
-  default:
+  case llvm::Triple::nvptx:
+  case llvm::Triple::nvptx64:
+  case llvm::Triple::xcore:
 return false;
+  default:
+return getTriple().getVendor() != llvm::Triple::Myriad;
   }
 }
 

diff  --git a/clang/test/Driver/integrated-as.c 
b/clang/test/Driver/integrated-as.c
index dfd669a626bd..55334ed71e59 100644
--- a/clang/test/Driver/integrated-as.c
+++ b/clang/test/Driver/integrated-as.c
@@ -12,5 +12,4 @@
 
 // NOFIAS-NOT: cc1as
 // NOFIAS: -cc1
-// NOFIAS: "-fno-verbose-asm"
 // NOFIAS: -no-integrated-as



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

[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/test/SemaCXX/type-traits.cpp:2886-2889
+struct UnnamedEmptyBitfield {
+  int named;
+  int : 0;
+};

royjacobson wrote:
> royjacobson wrote:
> > shafik wrote:
> > > aaron.ballman wrote:
> > > > I think there's one more test to add:
> > > > ```
> > > > struct UnnamedEmptyBitfieldSplit {
> > > >   short named;
> > > >   int : 0;
> > > >   short also_named;
> > > > };
> > > > static_assert(sizeof(UnnamedEmptyBitfieldSplit) != (sizeof(short) * 2));
> > > > static_assert(!has_unique_object_representations::value,
> > > >  "Bitfield padding");
> > > > ```
> > > Do we also want to check packed structs as well?
> > Do you have a test case where it would matter? Apparently `packed` is 
> > specified to ignore zero width bit fields.
> > 
> small ping :) @shafik 
Apologies, I just realized you thought I meant another zero sized bit-field 
case whereas I meant just test packed in general w/ non-zero sized bit-fields. 
The result should match for unpacked but it would be good to verify considering 
we seem to always get bitten by cases we don't cover.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145852

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

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added inline comments.



Comment at: clang/lib/Lex/ModuleMap.cpp:935
+  // with any legal user-defined module name).
+  StringRef IName = ".ImplementationUnit";
+  assert(!Modules[IName] && "multiple implementation units?");

nit: It should more consistent to use ``.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126959

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

[PATCH] D146758: Fix codegen for coroutine with function-try-block

[Chuanqi Xu via Phabricator via cfe-commits]

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

Since the patch itself is  good and not large. Let me handle the trivial 
refactoring later.




Comment at: clang/lib/CodeGen/CGCoroutine.cpp:724-730
+  Stmt *BodyStmt = S.getBody();
+  CompoundStmt *Body = dyn_cast(BodyStmt);
+  if (Body == nullptr) {
+Body =
+CompoundStmt::Create(getContext(), {BodyStmt}, FPOptionsOverride(),
+ SourceLocation(), SourceLocation());
+  }

MatzeB wrote:
> ChuanqiXu wrote:
> > MatzeB wrote:
> > > ChuanqiXu wrote:
> > > > Can we try to move the logic to `CoroutineStmtBuilder`? That makes me 
> > > > feel better. And it will be helpful to add a comment to tell that we're 
> > > > handling the case the function body is function-try-block.
> > > I'll add a detailed comment. But would you be fine leaving the statements 
> > > here as-is? The logic only makes sense in the context of using the `Body` 
> > > to create a `CXXTryStmt` below (it's really an effect of `CXXTryStmt` 
> > > only accepting CompountStmt operands).
> > It looks like you didn't address the comments. Would you like to address 
> > it? I don't mind to address it later myself.
> Did you mean to create a new function named `CoroutineStmtBuilder` like I did 
> now?
> Did you mean to create a new function named CoroutineStmtBuilder like I did 
> now?

No, I mean we should construct this in Sema.

> Putting an assert here feels unnecessary and may be in the way if in the 
> future we ever allow other types of single-statement function bodies.

Personally I prefer the more precise style.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146758

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

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

> That's a good question. I haven't looked into this issue deep enough, but I 
> think using -fexceptions requires using delayed diagnostics to avoid false 
> diagnostics during host code analysis.

I am assuming you mean `-fno-exceptions` (or, in `clang -cc1`, the absence of 
`-fexceptions`)? This is a good point. Delayed diagnostics would probably be 
good in general: we currently already emit warnings for host code when 
compiling for device, but as long as the generated warnings are identical for 
the host code as when compiling for host, it is easy enough to ignore; if the 
device compilation were to result in additional warnings for host code, and 
those warnings are incorrect, that would be a rather poor user experience. That 
sounds like a good additional reason to do it the way I had done here, the way 
the existing code comment had indicated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147097

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

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

@hvdijk, thanks a lot for fixing this.

In D147097#4229121 , @hvdijk wrote:

> Is the rationale I gave in the description correct, or would it be better for 
> SYCL device code to unconditionally build without `-fexceptions` and get the 
> `nounwind` attribute added that way?

That's a good question. I haven't looked into this issue deep enough, but I 
think using `-fexceptions` requires using delayed diagnostics to avoid false 
diagnostics during host code analysis. 
Anyway, all GPU offloading single-source modes have the same restriction and 
design and we better have unified solution whether it's using `-fexceptions` or 
adding `nounwind` attribute in CodeGen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147097

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

[PATCH] D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D146358#4229120 , @hazohelet wrote:

> In D146358#4227938 , @cjdb wrote:
>
>> In D146358#4204412 , @tbaeder 
>> wrote:
>>
>>> "subobject named 'foo'" sounds a bit weird to me, I'd expect just 
>>> "subobject 'foo'", but that's just a suggestion and I'll wait for a native 
>>> spearker to chime in on this.
>>
>> My expert brain likes `subobject of type 'T'`, but it's very wordy, and 
>> potentially not useful additional info. I'm partial to `subobject 'T'` due 
>> to it being the thing we're more likely to say in conversation (e.g. "that 
>> `int` isn't initialised"). I've also put out a mini-survey on the #include 
>>  Discord server, and will update in a few hours.
>
> Hi, @cjdb thanks for your feedback and the mini-survey.
> The objective of this patch is to print the subobject's name instead of its 
> type when it is not initialized in constexpr variable initializations. Thus, 
> I think it would be appropriate to add some more options to the survey like 
> "subobject named 'foo' is not initialized" and "subobject 'foo' is not 
> initialized" when the code looks like the following:
>
>   template 
>   struct F {
>   T foo;
>   constexpr F(){}
>   };
>   constexpr F f;

Ah, in that case, `subobject 'foo'` is better than `subobject named 'foo'` to 
me.


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

https://reviews.llvm.org/D146358

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

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

Is the rationale I gave in the description correct, or would it be better for 
SYCL device code to unconditionally build without `-fexceptions` and get the 
`nounwind` attribute added that way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147097

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

[PATCH] D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet added a comment.

In D146358#4227938 , @cjdb wrote:

> In D146358#4204412 , @tbaeder wrote:
>
>> "subobject named 'foo'" sounds a bit weird to me, I'd expect just "subobject 
>> 'foo'", but that's just a suggestion and I'll wait for a native spearker to 
>> chime in on this.
>
> My expert brain likes `subobject of type 'T'`, but it's very wordy, and 
> potentially not useful additional info. I'm partial to `subobject 'T'` due to 
> it being the thing we're more likely to say in conversation (e.g. "that `int` 
> isn't initialised"). I've also put out a mini-survey on the #include  
> Discord server, and will update in a few hours.

Hi, @cjdb thanks for your feedback and the mini-survey.
The objective of this patch is to print the subobject's name instead of its 
type when it is not initialized in constexpr variable initializations. Thus, I 
think it would be appropriate to add some more options to the survey like 
"subobject named 'foo' is not initialized" and "subobject 'foo' is not 
initialized" when the code looks like the following:

  template 
  struct F {
T foo;
constexpr F(){}
  };
  constexpr F f;


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

https://reviews.llvm.org/D146358

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

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk created this revision.
hvdijk added a reviewer: bader.
hvdijk added a project: clang.
Herald added subscribers: Naghasan, Anastasia, ebevhan, yaxunl.
Herald added a project: All.
hvdijk requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, jplehr, sstefan1.

Like CUDA and OpenCL, the SYCL specification says that throwing and
catching exceptions in device functions is not supported, so this change
extends the logic for adding the NoUnwind attribute to SYCL.

The existing convergent.cpp test, which tests that the convergent
attribute is added to functions by default, is renamed and reused to
test that the nounwind attribute is added by default. This test now has
-fexceptions added to it, which the driver adds by default as well.

The obvious question here is why not simply change the driver to remove
-fexceptions. This change follows the direction given by the TODO
comment because removing -fexceptions would also disable the
__EXCEPTIONS macro, which should reflect whether exceptions are enabled
on the host, rather than on the device, to avoid conflicts in types
shared between host and device.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147097

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenSYCL/convergent.cpp
  clang/test/CodeGenSYCL/function-attrs.cpp


Index: clang/test/CodeGenSYCL/function-attrs.cpp
===
--- clang/test/CodeGenSYCL/function-attrs.cpp
+++ clang/test/CodeGenSYCL/function-attrs.cpp
@@ -1,9 +1,10 @@
 // RUN: %clang_cc1 -fsycl-is-device -emit-llvm -disable-llvm-passes \
-// RUN:  -triple spir64 -emit-llvm %s -o - | FileCheck %s
+// RUN:  -triple spir64 -fexceptions -emit-llvm %s -o - | FileCheck %s
 
-// CHECK-DAG: Function Attrs:
-// CHECK-DAG-SAME: convergent
-// CHECK-DAG-NEXT: define void @_Z3foov
+// CHECK: Function Attrs:
+// CHECK-SAME: convergent
+// CHECK-SAME: nounwind
+// CHECK-NEXT: define dso_local spir_func void @_Z3foov
 void foo() {
   int a = 1;
 }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1971,10 +1971,9 @@
   }
 
   // TODO: NoUnwind attribute should be added for other GPU modes HIP,
-  // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
-  // code.
+  // OpenMP offload. AFAIK, neither of them support exceptions in device code.
   if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) ||
-  getLangOpts().OpenCL) {
+  getLangOpts().OpenCL || getLangOpts().SYCLIsDevice) {
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
   }
 


Index: clang/test/CodeGenSYCL/function-attrs.cpp
===
--- clang/test/CodeGenSYCL/function-attrs.cpp
+++ clang/test/CodeGenSYCL/function-attrs.cpp
@@ -1,9 +1,10 @@
 // RUN: %clang_cc1 -fsycl-is-device -emit-llvm -disable-llvm-passes \
-// RUN:  -triple spir64 -emit-llvm %s -o - | FileCheck %s
+// RUN:  -triple spir64 -fexceptions -emit-llvm %s -o - | FileCheck %s
 
-// CHECK-DAG: Function Attrs:
-// CHECK-DAG-SAME: convergent
-// CHECK-DAG-NEXT: define void @_Z3foov
+// CHECK: Function Attrs:
+// CHECK-SAME: convergent
+// CHECK-SAME: nounwind
+// CHECK-NEXT: define dso_local spir_func void @_Z3foov
 void foo() {
   int a = 1;
 }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1971,10 +1971,9 @@
   }
 
   // TODO: NoUnwind attribute should be added for other GPU modes HIP,
-  // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
-  // code.
+  // OpenMP offload. AFAIK, neither of them support exceptions in device code.
   if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) ||
-  getLangOpts().OpenCL) {
+  getLangOpts().OpenCL || getLangOpts().SYCLIsDevice) {
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:125
 
+  // The `-mroptr` option places constants in RO sections as much as possible.
+  // Then `-bforceimprw` changes such sections to RW if they contain imported

Old option name in comment text.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1965
+// therefore, we require that separate data sections
+// are used when `-mroptr` is in effect. We respect the setting of
+// data-sections since we have not found reasons to do otherwise that

Old option name in comment text.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144190

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

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:317
+  ``-fno-data-sections``. When ``-mxcoff-roptr`` is in effect at link time,
+  read-only  data sections with relocatable address values that resolve to
+  imported symbols are made writable.

Two spaces => one space



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:718-721
+  // On AIX, data_sections is on by default. We only need to check
+  // if data_sections is explicitly turned off.
+  if (Args.hasArg(options::OPT_fno_data_sections))
+D.Diag(diag::err_roptr_requires_data_sections);

Should use `hasFlag` to check if data_sections is off.

As background:
The logic near line 699 says to add `-data-sections=1` if data_sections is on 
and, if data_sections is off, only add `-data-sections=0` if 
`-fno-data-sections` was explicitly present. This allows the LTO default for 
data_sections to be a separate policy from the non-LTO default.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144190

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

[PATCH] D146678: Summary: Fix ArgsAsWritten being null for ConceptSpecializationExpr in certain circumstances when parsing ASTs

[Walter Gray via Phabricator via cfe-commits]

yeswalrus added a comment.

In D146678#4220408 , @erichkeane 
wrote:

> In D146678#4220360 , @yeswalrus 
> wrote:
>
>> -I can handle it. -
>> Edit: I cannot handle it, I don't have commit access. If you could commit to 
>> both main and the 16.x lines with the attribution to Walter Gray 
>>  that would be much appreciated. Thanks!
>
> You already have commit rights?  Ok then.  In order to get this merged into 
> the 16 release, you need to create a github issue to cherry-pick it, and add 
> it to the https://github.com/llvm/llvm-project/milestone/21 milestone.

I think there was a mixup. While I initially thought I could handle it, I do 
not in fact have commit rights.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146678

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

[PATCH] D146678: Summary: Fix ArgsAsWritten being null for ConceptSpecializationExpr in certain circumstances when parsing ASTs

[Walter Gray via Phabricator via cfe-commits]

yeswalrus updated this revision to Diff 509175.
yeswalrus retitled this revision from "Fix unexpected nullptr in 
ConceptSpecializationExpr's ArgsAsWritten field" to "Summary: Fix ArgsAsWritten 
being null for ConceptSpecializationExpr in certain circumstances when parsing 
ASTs".
yeswalrus added a comment.

Fix Nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146678

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ExprConcepts.cpp


Index: clang/lib/AST/ExprConcepts.cpp
===
--- clang/lib/AST/ExprConcepts.cpp
+++ clang/lib/AST/ExprConcepts.cpp
@@ -100,9 +100,9 @@
 ImplicitConceptSpecializationDecl *SpecDecl,
 const ConstraintSatisfaction *Satisfaction, bool Dependent,
 bool ContainsUnexpandedParameterPack) {
-  return new (C)
-  ConceptSpecializationExpr(C, NamedConcept, ArgsAsWritten, SpecDecl, 
Satisfaction,
-Dependent, ContainsUnexpandedParameterPack);
+  return new (C) ConceptSpecializationExpr(C, NamedConcept, ArgsAsWritten,
+   SpecDecl, Satisfaction, Dependent,
+   ContainsUnexpandedParameterPack);
 }
 
 const TypeConstraint *
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -840,9 +840,8 @@
   CSE->getNamedConcept()->getLocation(), NewConverted);
 
   Expr *NewIDC = ConceptSpecializationExpr::Create(
-  C, CSE->getNamedConcept(), CSE->getTemplateArgsAsWritten(),
-  CSD, nullptr, CSE->isInstantiationDependent(),
-  CSE->containsUnexpandedParameterPack());
+  C, CSE->getNamedConcept(), CSE->getTemplateArgsAsWritten(), CSD, 
/*Satisfaction=*/nullptr,
+  CSE->isInstantiationDependent(), CSE->containsUnexpandedParameterPack());
 
   if (auto *OrigFold = dyn_cast(IDC))
 NewIDC = new (C) CXXFoldExpr(


Index: clang/lib/AST/ExprConcepts.cpp
===
--- clang/lib/AST/ExprConcepts.cpp
+++ clang/lib/AST/ExprConcepts.cpp
@@ -100,9 +100,9 @@
 ImplicitConceptSpecializationDecl *SpecDecl,
 const ConstraintSatisfaction *Satisfaction, bool Dependent,
 bool ContainsUnexpandedParameterPack) {
-  return new (C)
-  ConceptSpecializationExpr(C, NamedConcept, ArgsAsWritten, SpecDecl, Satisfaction,
-Dependent, ContainsUnexpandedParameterPack);
+  return new (C) ConceptSpecializationExpr(C, NamedConcept, ArgsAsWritten,
+   SpecDecl, Satisfaction, Dependent,
+   ContainsUnexpandedParameterPack);
 }
 
 const TypeConstraint *
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -840,9 +840,8 @@
   CSE->getNamedConcept()->getLocation(), NewConverted);
 
   Expr *NewIDC = ConceptSpecializationExpr::Create(
-  C, CSE->getNamedConcept(), CSE->getTemplateArgsAsWritten(),
-  CSD, nullptr, CSE->isInstantiationDependent(),
-  CSE->containsUnexpandedParameterPack());
+  C, CSE->getNamedConcept(), CSE->getTemplateArgsAsWritten(), CSD, /*Satisfaction=*/nullptr,
+  CSE->isInstantiationDependent(), CSE->containsUnexpandedParameterPack());
 
   if (auto *OrigFold = dyn_cast(IDC))
 NewIDC = new (C) CXXFoldExpr(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146758: Fix codegen for coroutine with function-try-block

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added inline comments.



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:724-730
+  Stmt *BodyStmt = S.getBody();
+  CompoundStmt *Body = dyn_cast(BodyStmt);
+  if (Body == nullptr) {
+Body =
+CompoundStmt::Create(getContext(), {BodyStmt}, FPOptionsOverride(),
+ SourceLocation(), SourceLocation());
+  }

ChuanqiXu wrote:
> Can we try to move the logic to `CoroutineStmtBuilder`? That makes me feel 
> better. And it will be helpful to add a comment to tell that we're handling 
> the case the function body is function-try-block.
I'll add a detailed comment. But would you be fine leaving the statements here 
as-is? The logic only makes sense in the context of using the `Body` to create 
a `CXXTryStmt` below (it's really an effect of `CXXTryStmt` only accepting 
CompountStmt operands).



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:724-730
+  Stmt *BodyStmt = S.getBody();
+  CompoundStmt *Body = dyn_cast(BodyStmt);
+  if (Body == nullptr) {
+Body =
+CompoundStmt::Create(getContext(), {BodyStmt}, FPOptionsOverride(),
+ SourceLocation(), SourceLocation());
+  }

ChuanqiXu wrote:
> MatzeB wrote:
> > ChuanqiXu wrote:
> > > Can we try to move the logic to `CoroutineStmtBuilder`? That makes me 
> > > feel better. And it will be helpful to add a comment to tell that we're 
> > > handling the case the function body is function-try-block.
> > I'll add a detailed comment. But would you be fine leaving the statements 
> > here as-is? The logic only makes sense in the context of using the `Body` 
> > to create a `CXXTryStmt` below (it's really an effect of `CXXTryStmt` only 
> > accepting CompountStmt operands).
> It looks like you didn't address the comments. Would you like to address it? 
> I don't mind to address it later myself.
Did you mean to create a new function named `CoroutineStmtBuilder` like I did 
now?



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:726
+  CompoundStmt *Body = dyn_cast(BodyStmt);
+  if (Body == nullptr) {
+Body =

ChuanqiXu wrote:
> It reads better to specify the potential type for Body.
I will mention a single-statement body as an example in the comment now. 
Putting an `assert` here feels unnecessary and may be in the way if in the 
future we ever allow other types of single-statement function bodies.



Comment at: clang/test/CodeGenCoroutines/coro-function-try-block.cpp:21-23
+// CHECK-LABEL: define{{.*}} void @_Z1fv(
+// CHECK: call void 
@_ZNSt13suspend_never13await_suspendESt16coroutine_handleIvE(
+// CHECK: call void @_ZN4task12promise_type11return_voidEv(

ChuanqiXu wrote:
> I expect to see the nested try statements in the case.
This was mostly meant as a test for "does not crash the compiler", so it is 
just checking some parts of the output without caring too much what they are 
specifically.

I cannot test for nested try statements, because in llvm-ir those are already 
lowered to unwind tables and landing pad blocks, but there's neither a "try" 
statement nor an immediate nesting I could test for.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146758

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

[PATCH] D146758: Fix codegen for coroutine with function-try-block

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 509173.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146758

Files:
  clang/include/clang/AST/StmtCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/StmtCXX.cpp
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CodeGenCoroutines/coro-function-try-block.cpp

Index: clang/test/CodeGenCoroutines/coro-function-try-block.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/coro-function-try-block.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \
+// RUN:-disable-llvm-passes %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct task {
+  struct promise_type {
+task get_return_object();
+std::suspend_never initial_suspend();
+std::suspend_never final_suspend() noexcept;
+void return_void();
+void unhandled_exception() noexcept;
+  };
+};
+
+task f() try {
+  co_return;
+} catch(...) {
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z1fv(
+// CHECK: call void @_ZNSt13suspend_never13await_suspendESt16coroutine_handleIvE(
+// CHECK: call void @_ZN4task12promise_type11return_voidEv(
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -4546,7 +4546,8 @@
 
   FSI->setHasCXXTry(TryLoc);
 
-  return CXXTryStmt::Create(Context, TryLoc, TryBlock, Handlers);
+  return CXXTryStmt::Create(Context, TryLoc, cast(TryBlock),
+Handlers);
 }
 
 StmtResult Sema::ActOnSEHTryBlock(bool IsCXXTry, SourceLocation TryLoc,
Index: clang/lib/CodeGen/CGCoroutine.cpp
===
--- clang/lib/CodeGen/CGCoroutine.cpp
+++ clang/lib/CodeGen/CGCoroutine.cpp
@@ -593,6 +593,18 @@
   CGF.EmitStmt(OnFallthrough);
 }
 
+static CompoundStmt *CoroutineStmtBuilder(ASTContext ,
+  const CoroutineBodyStmt ) {
+  Stmt *Stmt = S.getBody();
+  if (CompoundStmt *Body = dyn_cast(Stmt))
+return Body;
+  // We are about to create a `CXXTryStmt` which requires a `CompoundStmt`.
+  // If the function body is not a `CompoundStmt` yet then we have to create
+  // a new one. This happens for cases like the "function-try-block" syntax.
+  return CompoundStmt::Create(Context, {Stmt}, FPOptionsOverride(),
+  SourceLocation(), SourceLocation());
+}
+
 void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt ) {
   auto *NullPtr = llvm::ConstantPointerNull::get(Builder.getInt8PtrTy());
   auto  = CGM.getContext().getTargetInfo();
@@ -721,8 +733,8 @@
   auto Loc = S.getBeginLoc();
   CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr,
  CurCoro.Data->ExceptionHandler);
-  auto *TryStmt =
-  CXXTryStmt::Create(getContext(), Loc, S.getBody(), );
+  CompoundStmt *Body = CoroutineStmtBuilder(getContext(), S);
+  auto *TryStmt = CXXTryStmt::Create(getContext(), Loc, Body, );
 
   EnterCXXTryStmt(*TryStmt);
   emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
Index: clang/lib/AST/StmtCXX.cpp
===
--- clang/lib/AST/StmtCXX.cpp
+++ clang/lib/AST/StmtCXX.cpp
@@ -23,7 +23,8 @@
 }
 
 CXXTryStmt *CXXTryStmt::Create(const ASTContext , SourceLocation tryLoc,
-   Stmt *tryBlock, ArrayRef handlers) {
+   CompoundStmt *tryBlock,
+   ArrayRef handlers) {
   const size_t Size = totalSizeToAlloc(handlers.size() + 1);
   void *Mem = C.Allocate(Size, alignof(CXXTryStmt));
   return new (Mem) CXXTryStmt(tryLoc, tryBlock, handlers);
@@ -36,7 +37,7 @@
   return new (Mem) CXXTryStmt(Empty, numHandlers);
 }
 
-CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, Stmt *tryBlock,
+CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, CompoundStmt *tryBlock,
ArrayRef handlers)
 : Stmt(CXXTryStmtClass), TryLoc(tryLoc), NumHandlers(handlers.size()) {
   Stmt **Stmts = getStmts();
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -6793,8 +6793,8 @@
   return ToHandlerOrErr.takeError();
   }
 
-  return CXXTryStmt::Create(
-  Importer.getToContext(), *ToTryLocOrErr,*ToTryBlockOrErr, ToHandlers);
+  return CXXTryStmt::Create(Importer.getToContext(), *ToTryLocOrErr,
+cast(*ToTryBlockOrErr), ToHandlers);
 }
 
 ExpectedStmt ASTNodeImporter::VisitCXXForRangeStmt(CXXForRangeStmt *S) {
Index: clang/include/clang/AST/StmtCXX.h
===
--- clang/include/clang/AST/StmtCXX.h
+++ 

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:647
+def err_roptr_requires_data_sections: Error<"-mxcoff-roptr is supported only 
with -fdata-sections">;
+def err_roptr_cannot_build_shared: Error<"-mxcoff-roptr is not suppored with 
-shared">;
 

Typo.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5258-5259
+if (HasRoptr) {
+  if (Args.hasArg(options::OPT_shared))
+D.Diag(diag::err_roptr_cannot_build_shared);
+

I think this belongs in the code for AIX linker jobs.



Comment at: clang/test/Driver/ppc-roptr.c:37
+// LINK: "-bforceimprw"
+// LTO_ROPTR: "-bplugin_opt:-mxcoff-roptr"
+// NO_ROPTR-NOT: "-mxcoff-roptr"

This needs the backend option also renamed. Has a commit for that landed yet?



Comment at: clang/test/Driver/ppc-roptr.c:15
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mroptr -flto %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=LTO_ROPTR
+// RUN: touch %t.o

This is compile+LTO link. We can also add `ROPTR,LINK` prefixes here, right?



Comment at: clang/test/Driver/ppc-roptr.c:24
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_NOROPTR_ERR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mroptr -shared \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=SHARED_ERR

Please make this case pure-link. The diagnostic is most relevant on the link 
step because that is when `-shared` takes effect.



Comment at: clang/test/Driver/ppc-roptr.c:26
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=SHARED_ERR
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mroptr -flto \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR

Please make this case pure-link; otherwise, we will pass the test simply 
because the diagnostic is also implemented for compiling-from-source.



Comment at: clang/test/Driver/ppc-roptr.c:28
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mroptr -flto 
-fno-data-sections \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR

Same comment.



Comment at: clang/test/Driver/ppc-roptr.c:30
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mno-roptr -flto \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_NOROPTR_ERR

Same comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144190

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

[PATCH] D147070: Improve requirement clause limitation on non templated function

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/test/CXX/dcl.decl/dcl.decl.general/p4-20.cpp:27
+
+namespace GH61748 {
+template

Maybe a union case just for completeness:

```
template 
union U {
void f() requires true;
};
```


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

https://reviews.llvm.org/D147070

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

[PATCH] D146465: [clang] Fix 2 bugs with parenthesized aggregate initialization

[Alan Zhao via Phabricator via cfe-commits]

ayzhao updated this revision to Diff 509160.
ayzhao marked an inline comment as done.
ayzhao added a comment.

fix stray whitespace change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146465

Files:
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/CodeGen/paren-list-agg-init.cpp
  clang/test/SemaCXX/paren-list-agg-init.cpp

Index: clang/test/SemaCXX/paren-list-agg-init.cpp
===
--- clang/test/SemaCXX/paren-list-agg-init.cpp
+++ clang/test/SemaCXX/paren-list-agg-init.cpp
@@ -65,7 +65,7 @@
 void bar() {
   T t = 0;
   A a(CH, 1.1); // OK; C++ paren list constructors are supported in semantic tree transformations.
-  // beforecxx20-warning@-1 {{aggregate initialization of type 'A' from a parenthesized list of values is a C++20 extension}}
+  // beforecxx20-warning@-1 2{{aggregate initialization of type 'A' from a parenthesized list of values is a C++20 extension}}
 }
 
 template 
@@ -139,7 +139,8 @@
   constexpr F f2(1, 1); // OK: f2.b is initialized by a constant expression.
   // beforecxx20-warning@-1 {{aggregate initialization of type 'const F' from a parenthesized list of values is a C++20 extension}}
 
-  bar();
+  bar();
+  // beforecxx20-note@-1 {{in instantiation of function template specialization 'bar' requested here}}
 
   G g('b', 'b');
   // beforecxx20-warning@-1 {{aggregate initialization of type 'G' from a parenthesized list of values is a C++20 extension}}
Index: clang/test/CodeGen/paren-list-agg-init.cpp
===
--- clang/test/CodeGen/paren-list-agg-init.cpp
+++ clang/test/CodeGen/paren-list-agg-init.cpp
@@ -69,6 +69,27 @@
   char b;
 };
 
+
+namespace gh61145 {
+  // CHECK-DAG: [[STRUCT_VEC:%.*]] = type { i8 }
+  struct Vec {
+Vec();
+Vec(Vec&&);
+~Vec();
+  };
+
+  // CHECK-DAG: [[STRUCT_S1:%.*]] = type { [[STRUCT_VEC]] }
+  struct S1 {
+Vec v;
+  };
+
+  // CHECK-DAG: [[STRUCT_S2:%.*]] = type { [[STRUCT_VEC]], i8 }
+  struct S2 {
+Vec v;
+char c;
+  };
+}
+
 // CHECK-DAG: [[A1:@.*a1.*]] = internal constant [[STRUCT_A]] { i8 3, double 2.00e+00 }, align 8
 constexpr A a1(3.1, 2.0);
 // CHECK-DAG: [[A2:@.*a2.*]] = internal constant [[STRUCT_A]] { i8 99, double 0.00e+00 }, align 8
@@ -349,3 +370,54 @@
 void foo19() {
   G g(2);
 }
+
+namespace gh61145 {
+  // a.k.a. void make1<0>()
+  // CHECK: define {{.*}} void @_ZN7gh611455make1ILi0EEEvv
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT: [[V:%.*v.*]] = alloca [[STRUCT_VEC]], align 1
+  // CHECK-NEXT: [[AGG_TMP_ENSURED:%.*agg.tmp.ensured.*]] = alloca [[STRUCT_S1]], align 1
+  // a.k.a. Vec::Vec()
+  // CHECK-NEXT: call void @_ZN7gh611453VecC1Ev(ptr noundef nonnull align 1 dereferenceable(1) [[V]])
+  // CHECK-NEXT: [[V1:%.*v1.*]] = getelementptr inbounds [[STRUCT_S1]], ptr [[AGG_TMP_ENSURED]], i32 0, i32 0
+  // a.k.a. Vec::Vec(Vec&&)
+  // CHECK-NEXT: call void @_ZN7gh611453VecC1EOS0_(ptr noundef nonnull align 1 dereferenceable(1) [[V1]], ptr noundef nonnull align 1 dereferenceable(1) [[V]])
+  // a.k.a. S1::~S1()
+  // CHECK-NEXT: call void @_ZN7gh611452S1D1Ev(ptr noundef nonnull align 1 dereferenceable(1) [[AGG_TMP_ENSURED]])
+  // a.k.a.Vec::~Vec()
+  // CHECK-NEXT: call void @_ZN7gh611453VecD1Ev(ptr noundef nonnull align 1 dereferenceable(1) [[V]])
+  // CHECK-NEXT: ret void
+  template 
+  void make1() {
+Vec v;
+S1((Vec&&) v);
+  }
+
+  // a.k.a. void make1<0>()
+  // CHECK: define {{.*}} void @_ZN7gh611455make2ILi0EEEvv
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT: [[V:%.*v.*]] = alloca [[STRUCT_VEC]], align 1
+  // CHECK-NEXT: [[AGG_TMP_ENSURED:%.*agg.tmp.ensured.*]] = alloca [[STRUCT_S2]], align 1
+  // a.k.a. Vec::Vec()
+  // CHECK-NEXT: call void @_ZN7gh611453VecC1Ev(ptr noundef nonnull align 1 dereferenceable(1) [[V]])
+  // CHECK-NEXT: [[V1:%.*v1.*]] = getelementptr inbounds [[STRUCT_S2]], ptr [[AGG_TMP_ENSURED]], i32 0, i32 0
+  // a.k.a. Vec::Vec(Vec&&)
+  // CHECK-NEXT: call void @_ZN7gh611453VecC1EOS0_(ptr noundef nonnull align 1 dereferenceable(1) [[V1]], ptr noundef nonnull align 1 dereferenceable(1) [[V]])
+  // CHECK-NEXT: [[C:%.*c.*]] = getelementptr inbounds [[STRUCT_S2]], ptr [[AGG_TMP_ENSURED]], i32 0, i32
+  // CHECK-NEXT: store i8 0, ptr [[C]], align 1
+  // a.k.a. S2::~S2()
+  // CHECK-NEXT: call void @_ZN7gh611452S2D1Ev(ptr noundef nonnull align 1 dereferenceable(2) [[AGG_TMP_ENSURED]])
+  // a.k.a. Vec::~Vec()
+  // CHECK-NEXT: call void @_ZN7gh611453VecD1Ev(ptr noundef nonnull align 1 dereferenceable(1) [[V]])
+  // CHECK-NEXT: ret void
+  template 
+  void make2() {
+Vec v;
+S2((Vec&&) v, 0);
+  }
+
+  void foo() {
+make1<0>();
+make2<0>();
+  }
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ 

[PATCH] D146465: [clang] Fix 2 bugs with parenthesized aggregate initialization

[Alan Zhao via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaExprCXX.cpp:1582-1596
+  MultiExprArg ExprsToPass;
+  if (Exprs.size() == 1 && isa(Exprs[0])) {
+// C++20 [expr.static.cast]p4:
+//   An expression E can be explicitly converted to a type T...if T is an
+//   aggregate type ([dcl.init.aggr]) having a first element x and there is
+//   an implicit conversion sequence from E to the type of x
+//

rsmith wrote:
> ayzhao wrote:
> > rsmith wrote:
> > > The input to this function should be a syntactic initializer such as a 
> > > `ParenListExpr`, not an already-type-checked semantic initializer such as 
> > > a `CXXParenListInitExpr`. The right place to do this unwrapping is in 
> > > `TreeTransform::TransformInitializer`, where we unwrap various other 
> > > kinds of semantic initializer.
> > >  The right place to do this unwrapping is in 
> > > `TreeTransform::TransformInitializer`, where we unwrap various other 
> > > kinds of semantic initializer.
> > 
> > So the `CXXParenListInitExpr` for `S` never gets passed to 
> > `TreeTransform::TransformInitializer` - rather, 
> > `TransformCXXParenListInitExpr(...)` calls `TransformExprs(...)` on its 
> > subexpressions. If the subexpression happens to be a `CXXConstructExpr`, 
> > then `TransformCXXConstructExpr` will call `TransformInitializer(...)`, 
> > which is [why we saw the CXXConstructExpr getting 
> > deleted](https://github.com/llvm/llvm-project/blob/4c483a046d2ff29ec2fd5bad6305f97424a2b880/clang/lib/Sema/TreeTransform.h#L13030-L13036)
> > 
> > Interestingly enough, if we initialize `S` with a braced init-list (i.e. 
> > `S{(V&&) Vec}`), we never end up calling `TransformCXXConstructExpr(...)` 
> > since [we revert to the syntatic 
> > form.](https://github.com/llvm/llvm-project/blob/4c483a046d2ff29ec2fd5bad6305f97424a2b880/clang/lib/Sema/TreeTransform.h#L11584-L11585).
> > 
> > > The input to this function should be a syntactic initializer such as a 
> > > `ParenListExpr`, not an already-type-checked semantic initializer such as 
> > > a `CXXParenListInitExpr`.
> > 
> > We can probably do this by returning a `ParenListExpr` in 
> > `TransformCXXParenListInitExpr(...)`. This does cause some problems though. 
> > If we use a `ParenListExpr`, how would we know when to bail out in line 
> > 1551 (`!isa(Exprs[0]) && 
> > !isa(Exprs[0])`)? Surely, not every instance of a 
> > `ParenListExpr` would be caused by a `CXXParenListInitExpr`, and if this 
> > were the case, that would be a pretty brittle assumption. Furthermore, we 
> > never create a `ParenListExpr` for a corresponding `CXXParenListInitExpr`.
> > 
> > As an alternative, we could probably do something similar to 
> > `InitListExpr`, which has separate semantic and syntactic forms. Would that 
> > work?
> > 
> > Also, in general, are the `TransformFooExpr(...)` functions supposed to 
> > return semantic or syntactic results? I seem to see a mix of both...
> OK, I've looked at this a bit more. It looks to me like there are problems in 
> two places:
> 
> 1) 
> [TreeTransform::RebuildCXXParenListInitExpr](https://github.com/llvm/llvm-project/blob/4c483a046d2ff29ec2fd5bad6305f97424a2b880/clang/lib/Sema/TreeTransform.h#L3889).
>  When transforming the subexpressions of a `CXXParenListInitExpr`, we call 
> `TransformExprs` passing `IsCall=true`, which means we call 
> `TransformInitializer` on each of them, which reverts them to syntactic form. 
> So we need to redo the semantic analysis for forming a `CXXParenListInitExpr` 
> here, and can't just create a new `CXXParenListInitExpr` node directly. 
> Probably the easiest way to handle that would be to produce a `ParenListExpr` 
> here instead.
> 
> 2) 
> [Expr::getSubExprAsWritten](https://github.com/llvm/llvm-project/blob/4c483a046d2ff29ec2fd5bad6305f97424a2b880/clang/lib/AST/Expr.cpp#L1978)
>  ([reached via 
> here](https://github.com/llvm/llvm-project/blob/4c483a046d2ff29ec2fd5bad6305f97424a2b880/clang/lib/Sema/TreeTransform.h#L12070))
>  is not properly handling `CXXParenListExpr` -- it should be stepping over it 
> to the single argument of the expression, which was the as-written 
> subexpression.
> 
> But looking more into (2), it seems like there's a representational oddity 
> here, too. We use a `CXXFunctionalCastExpr` wrapping a `CXXParenListExpr` to 
> model both the case of `Aggregate(a)` (which is a functional cast, equivalent 
> by definition to `(Aggregate)a`) and the case of `Aggregate(a, b)` (which is 
> //not// a functional cast). I don't think that's necessarily a problem -- we 
> also incorrectly use `CXXFunctionalCastExpr` wrapping an `InitListExpr` for 
> `Aggregate{a, b}` -- but it means that `getSubExprAsWritten` will need to be 
> careful to only unwrap in the case where the parenthesized aggregate 
> initialization expression had exactly one element, and it's not ideal from a 
> source fidelity perspective.
> 
> In the long 

[PATCH] D146465: [clang] Fix 2 bugs with parenthesized aggregate initialization

[Alan Zhao via Phabricator via cfe-commits]

ayzhao updated this revision to Diff 509158.
ayzhao added a comment.

use ParenListExpr and fix getSubExprAsWritten(...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146465

Files:
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/CodeGen/paren-list-agg-init.cpp
  clang/test/SemaCXX/paren-list-agg-init.cpp

Index: clang/test/SemaCXX/paren-list-agg-init.cpp
===
--- clang/test/SemaCXX/paren-list-agg-init.cpp
+++ clang/test/SemaCXX/paren-list-agg-init.cpp
@@ -65,7 +65,7 @@
 void bar() {
   T t = 0;
   A a(CH, 1.1); // OK; C++ paren list constructors are supported in semantic tree transformations.
-  // beforecxx20-warning@-1 {{aggregate initialization of type 'A' from a parenthesized list of values is a C++20 extension}}
+  // beforecxx20-warning@-1 2{{aggregate initialization of type 'A' from a parenthesized list of values is a C++20 extension}}
 }
 
 template 
@@ -139,7 +139,8 @@
   constexpr F f2(1, 1); // OK: f2.b is initialized by a constant expression.
   // beforecxx20-warning@-1 {{aggregate initialization of type 'const F' from a parenthesized list of values is a C++20 extension}}
 
-  bar();
+  bar();
+  // beforecxx20-note@-1 {{in instantiation of function template specialization 'bar' requested here}}
 
   G g('b', 'b');
   // beforecxx20-warning@-1 {{aggregate initialization of type 'G' from a parenthesized list of values is a C++20 extension}}
Index: clang/test/CodeGen/paren-list-agg-init.cpp
===
--- clang/test/CodeGen/paren-list-agg-init.cpp
+++ clang/test/CodeGen/paren-list-agg-init.cpp
@@ -69,6 +69,27 @@
   char b;
 };
 
+
+namespace gh61145 {
+  // CHECK-DAG: [[STRUCT_VEC:%.*]] = type { i8 }
+  struct Vec {
+Vec();
+Vec(Vec&&);
+~Vec();
+  };
+
+  // CHECK-DAG: [[STRUCT_S1:%.*]] = type { [[STRUCT_VEC]] }
+  struct S1 {
+Vec v;
+  };
+
+  // CHECK-DAG: [[STRUCT_S2:%.*]] = type { [[STRUCT_VEC]], i8 }
+  struct S2 {
+Vec v;
+char c;
+  };
+}
+
 // CHECK-DAG: [[A1:@.*a1.*]] = internal constant [[STRUCT_A]] { i8 3, double 2.00e+00 }, align 8
 constexpr A a1(3.1, 2.0);
 // CHECK-DAG: [[A2:@.*a2.*]] = internal constant [[STRUCT_A]] { i8 99, double 0.00e+00 }, align 8
@@ -349,3 +370,54 @@
 void foo19() {
   G g(2);
 }
+
+namespace gh61145 {
+  // a.k.a. void make1<0>()
+  // CHECK: define {{.*}} void @_ZN7gh611455make1ILi0EEEvv
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT: [[V:%.*v.*]] = alloca [[STRUCT_VEC]], align 1
+  // CHECK-NEXT: [[AGG_TMP_ENSURED:%.*agg.tmp.ensured.*]] = alloca [[STRUCT_S1]], align 1
+  // a.k.a. Vec::Vec()
+  // CHECK-NEXT: call void @_ZN7gh611453VecC1Ev(ptr noundef nonnull align 1 dereferenceable(1) [[V]])
+  // CHECK-NEXT: [[V1:%.*v1.*]] = getelementptr inbounds [[STRUCT_S1]], ptr [[AGG_TMP_ENSURED]], i32 0, i32 0
+  // a.k.a. Vec::Vec(Vec&&)
+  // CHECK-NEXT: call void @_ZN7gh611453VecC1EOS0_(ptr noundef nonnull align 1 dereferenceable(1) [[V1]], ptr noundef nonnull align 1 dereferenceable(1) [[V]])
+  // a.k.a. S1::~S1()
+  // CHECK-NEXT: call void @_ZN7gh611452S1D1Ev(ptr noundef nonnull align 1 dereferenceable(1) [[AGG_TMP_ENSURED]])
+  // a.k.a.Vec::~Vec()
+  // CHECK-NEXT: call void @_ZN7gh611453VecD1Ev(ptr noundef nonnull align 1 dereferenceable(1) [[V]])
+  // CHECK-NEXT: ret void
+  template 
+  void make1() {
+Vec v;
+S1((Vec&&) v);
+  }
+
+  // a.k.a. void make1<0>()
+  // CHECK: define {{.*}} void @_ZN7gh611455make2ILi0EEEvv
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT: [[V:%.*v.*]] = alloca [[STRUCT_VEC]], align 1
+  // CHECK-NEXT: [[AGG_TMP_ENSURED:%.*agg.tmp.ensured.*]] = alloca [[STRUCT_S2]], align 1
+  // a.k.a. Vec::Vec()
+  // CHECK-NEXT: call void @_ZN7gh611453VecC1Ev(ptr noundef nonnull align 1 dereferenceable(1) [[V]])
+  // CHECK-NEXT: [[V1:%.*v1.*]] = getelementptr inbounds [[STRUCT_S2]], ptr [[AGG_TMP_ENSURED]], i32 0, i32 0
+  // a.k.a. Vec::Vec(Vec&&)
+  // CHECK-NEXT: call void @_ZN7gh611453VecC1EOS0_(ptr noundef nonnull align 1 dereferenceable(1) [[V1]], ptr noundef nonnull align 1 dereferenceable(1) [[V]])
+  // CHECK-NEXT: [[C:%.*c.*]] = getelementptr inbounds [[STRUCT_S2]], ptr [[AGG_TMP_ENSURED]], i32 0, i32
+  // CHECK-NEXT: store i8 0, ptr [[C]], align 1
+  // a.k.a. S2::~S2()
+  // CHECK-NEXT: call void @_ZN7gh611452S2D1Ev(ptr noundef nonnull align 1 dereferenceable(2) [[AGG_TMP_ENSURED]])
+  // a.k.a. Vec::~Vec()
+  // CHECK-NEXT: call void @_ZN7gh611453VecD1Ev(ptr noundef nonnull align 1 dereferenceable(1) [[V]])
+  // CHECK-NEXT: ret void
+  template 
+  void make2() {
+Vec v;
+S2((Vec&&) v, 0);
+  }
+
+  void foo() {
+make1<0>();
+make2<0>();
+  }
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h

[PATCH] D146101: [clang-format] Add DesignatedInitializerIndentWidth option.

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1665-1669
+  const auto DesignatedInitializerIndentWidth =
+  Style.DesignatedInitializerIndentWidth < 0
+  ? Style.ContinuationIndentWidth
+  : Style.DesignatedInitializerIndentWidth;
+  NewIndent = CurrentState.LastSpace + DesignatedInitializerIndentWidth;

jp4a50 wrote:
> HazardyKnusperkeks wrote:
> > owenpan wrote:
> > > jp4a50 wrote:
> > > > owenpan wrote:
> > > > > owenpan wrote:
> > > > > > Using -1 to mean `ContinuationIndentWidth` is unnecessary and 
> > > > > > somewhat confusing IMO.
> > > > > Please disregard my comment above.
> > > > Just to make sure we are on the same page, does this mean that you are 
> > > > happy with the approach of using `-1` as a default value to indicate 
> > > > that `ContinuationIndentWidth` should be used?
> > > > 
> > > > I did initially consider using `std::optional` and using 
> > > > empty optional to indicate that `ContinuationIndentWidth` should be 
> > > > used but I saw that `PPIndentWidth` was using `-1` to default to using 
> > > > `IndentWidth` so I followed that precedent.
> > > Yep! I would prefer the `optional`, but as you pointed out, we already 
> > > got `PPIndentWidth`using `-1`.
> > From the C++ side I totally agree. One could use `value_or()`, which would 
> > make the code much more readable.
> > And just because `PPIndentWidth` is using -1 is no reason to repeat that, 
> > we could just as easily change `PPIndentWidht` to an optional.
> > 
> > But how would it look in yaml?
> In YAML we wouldn't need to support empty optional being *explicitly* 
> specified - it would just be the default.
> 
> So specifying `DesignatedInitializerIndentWidth: 4` would set the 
> `std::optional` to `4` but if `DesignatedInitializerIndentWidth` 
> was omitted from the YAML then the optional would simply not be set during 
> parsing.
> 
> Of course, if we were to change `PPIndentWidth` to work that way too, it 
> would technically be a breaking change because users may have *explicitly* 
> specified `-1` in their YAML.
> And just because `PPIndentWidth` is using -1 is no reason to repeat that, we 
> could just as easily change `PPIndentWidht` to an optional.

We would have to deal with backward compatibility to avoid regressions though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146101

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

[PATCH] D146669: [-Wunsafe-buffer-usage] Hide fixits/suggestions behind an extra flag, -fsafe-buffer-usage-suggestions.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

So in a nutshell, this is the intended behavior:

| code | diagnostic 
 | EmitFixits && EmitSuggestions | !EmitFixits && 
EmitSuggestions | !EmitFixits && !EmitSuggestions |
|  | 
---
 | - | -- | 
--- |
| `void foo(int *p)` { | note: 'p' declared here
 | yes   | yes  
  | no  |
| `  int *q;`  | warning: 'q' is unsafe pointer to raw buffer   
 | yes   | yes  
  | no  |
|  | note with fixit: change type of 'q' and 'p' to span to 
propagate bounds information | yes   | yes (no fixit)   
  | no  |
| `  q = p;`   | note: 'bounds information propagates from 'p' to 'q' 
here   | yes   | yes
| no  |
| `  q[5] = 10;`   | note: unsafe raw buffer operation here 
 | yes   | yes  
  | yes (warning)   |
|  | note: pass -fsafe-buffer-usage-suggestions to receive 
code hardening suggestions| no| no  
   | yes |
| `}`  |
 |   |  
  | |
|

The first mode, "**EmitFixits && EmitSuggestions**", is the default behavior 
before this patch, and the behavior under `-fsafe-buffer-usage-suggestions` 
after this patch. The third mode, "**!EmitFixits && !EmitSuggestions**", is the 
default behavior after the patch.

The second mode, "**!EmitFixits && EmitSuggestions**" (the special behavior 
under `-fno-diagnostics-fixit-info`) was implemented for two purposes:

- Recover some performance when `-fno-diagnostics-fixit-info` is passed (don't 
run the sophisticated fixit machinery if fixits are going to be discarded);
- As a possible workaround for potential crashes in the fixit machinery.

We initially hoped that this mode will make "**!EmitSuggestions**" unnecessary, 
but as the table above demonstrates, a very large portion of the fixit 
machine's logic is still being invoked for the purposes of building notes, even 
when fixits aren't attached. And we cannot really disable this logic under that 
compiler flag, given that  `-fno-diagnostics-fixit-info` already has a 
well-defined meaning which doesn't allow us to change the shape of the 
diagnostics in response. So we needed a new, more powerful flag, so here we 
are. I suspect that explicit support for `!EmitFixits` is no longer necessary. 
It is now useful for performance purposes only, and we don't any concrete data 
to support the claim that it's a valuable optimization, nor do we believe that 
this flag is ever intentionally passed in practice, so I guess I'll go ahead 
and remove it.


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

https://reviews.llvm.org/D146669

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

[PATCH] D146840: [AMDGPU] Replace target feature for global fadd32

[Anshil Gandhi 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 rGa955a3189637: [AMDGPU] Replace target feature for global 
fadd32 (authored by gandhi21299).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146840

Files:
  clang/include/clang/Basic/BuiltinsAMDGPU.def
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/CodeGenOpenCL/amdgpu-features.cl
  clang/test/CodeGenOpenCL/builtins-amdgcn-fp-atomics-gfx908-err.cl
  clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
  clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
  clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx940.cl

Index: clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx940.cl
===
--- clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx940.cl
+++ clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx940.cl
@@ -64,3 +64,9 @@
 void test_local_add_2f16_noret(__local half2 *addr, half2 x) {
   __builtin_amdgcn_ds_atomic_fadd_v2f16(addr, x);
 }
+
+// CHECK-LABEL: @test_global_add_f32
+// CHECK: call float @llvm.amdgcn.global.atomic.fadd.f32.p1.f32(ptr addrspace(1) %{{.*}}, float %{{.*}})
+void test_global_add_f32(float *rtn, global float *addr, float x) {
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f32(addr, x);
+}
Index: clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
===
--- clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
+++ clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
@@ -115,3 +115,9 @@
   float *rtn;
   *rtn = __builtin_amdgcn_ds_atomic_fadd_f32(addr, x);
 }
+
+// CHECK-LABEL: @test_global_add_f32
+// CHECK: call float @llvm.amdgcn.global.atomic.fadd.f32.p1.f32(ptr addrspace(1) %{{.*}}, float %{{.*}})
+void test_global_add_f32(float *rtn, global float *addr, float x) {
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f32(addr, x);
+}
Index: clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
===
--- clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
+++ clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
@@ -43,3 +43,9 @@
 void test_s_wait_event_export_ready() {
   __builtin_amdgcn_s_wait_event_export_ready();
 }
+
+// CHECK-LABEL: @test_global_add_f32
+// CHECK: call float @llvm.amdgcn.global.atomic.fadd.f32.p1.f32(ptr addrspace(1) %{{.*}}, float %{{.*}})
+void test_global_add_f32(float *rtn, global float *addr, float x) {
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f32(addr, x);
+}
Index: clang/test/CodeGenOpenCL/builtins-amdgcn-fp-atomics-gfx908-err.cl
===
--- clang/test/CodeGenOpenCL/builtins-amdgcn-fp-atomics-gfx908-err.cl
+++ clang/test/CodeGenOpenCL/builtins-amdgcn-fp-atomics-gfx908-err.cl
@@ -11,7 +11,7 @@
   float *fp_rtn;
   double *rtn;
   *half_rtn = __builtin_amdgcn_global_atomic_fadd_v2f16(addrh2, xh2); // expected-error{{'__builtin_amdgcn_global_atomic_fadd_v2f16' needs target feature atomic-buffer-global-pk-add-f16-insts}}
-  *fp_rtn = __builtin_amdgcn_global_atomic_fadd_f32(addr, x); // expected-error{{'__builtin_amdgcn_global_atomic_fadd_f32' needs target feature gfx90a-insts}}
+  *fp_rtn = __builtin_amdgcn_global_atomic_fadd_f32(addr, x); // expected-error{{'__builtin_amdgcn_global_atomic_fadd_f32' needs target feature atomic-fadd-rtn-insts}}
   *rtn = __builtin_amdgcn_global_atomic_fadd_f64(addr, x); // expected-error{{'__builtin_amdgcn_global_atomic_fadd_f64' needs target feature gfx90a-insts}}
   *rtn = __builtin_amdgcn_global_atomic_fmax_f64(addr, x); // expected-error{{'__builtin_amdgcn_global_atomic_fmax_f64' needs target feature gfx90a-insts}}
   *rtn = __builtin_amdgcn_global_atomic_fmin_f64(addr, x); // expected-error{{'__builtin_amdgcn_global_atomic_fmin_f64' needs target feature gfx90a-insts}}
Index: clang/test/CodeGenOpenCL/amdgpu-features.cl
===
--- clang/test/CodeGenOpenCL/amdgpu-features.cl
+++ clang/test/CodeGenOpenCL/amdgpu-features.cl
@@ -72,9 +72,9 @@
 // GFX906: "target-features"="+16-bit-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
 // GFX908: "target-features"="+16-bit-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
 // GFX909: "target-features"="+16-bit-insts,+ci-insts,+dpp,+gfx8-insts,+gfx9-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
-// GFX90A: 

[clang] a955a31 - [AMDGPU] Replace target feature for global fadd32

[Anshil Gandhi via cfe-commits]

Author: Anshil Gandhi
Date: 2023-03-28T15:58:30-06:00
New Revision: a955a31896370b67c6490251eca0095295d55f1f

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

LOG: [AMDGPU] Replace target feature for global fadd32

Change target feature of __builtin_amdgcn_global_atomic_fadd_f32
to atomic-fadd-rtn-insts. Enable atomic-fadd-rtn-insts for gfx90a,
gfx940 and gfx1100 as they all support the return variant of
`global_atomic_add_f32`.

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

Reviewed By: rampitec

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsAMDGPU.def
clang/lib/Basic/Targets/AMDGPU.cpp
clang/test/CodeGenOpenCL/amdgpu-features.cl
clang/test/CodeGenOpenCL/builtins-amdgcn-fp-atomics-gfx908-err.cl
clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx940.cl

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsAMDGPU.def 
b/clang/include/clang/Basic/BuiltinsAMDGPU.def
index 965bd97a97d79..0196100cccac5 100644
--- a/clang/include/clang/Basic/BuiltinsAMDGPU.def
+++ b/clang/include/clang/Basic/BuiltinsAMDGPU.def
@@ -214,7 +214,7 @@ TARGET_BUILTIN(__builtin_amdgcn_perm, "UiUiUiUi", "nc", 
"gfx8-insts")
 TARGET_BUILTIN(__builtin_amdgcn_fmed3h, "", "nc", "gfx9-insts")
 
 TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_f64, "dd*1d", "t", 
"gfx90a-insts")
-TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_f32, "ff*1f", "t", 
"gfx90a-insts")
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_f32, "ff*1f", "t", 
"atomic-fadd-rtn-insts")
 TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_v2f16, "V2hV2h*1V2h", "t", 
"atomic-buffer-global-pk-add-f16-insts")
 TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fmin_f64, "dd*1d", "t", 
"gfx90a-insts")
 TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fmax_f64, "dd*1d", "t", 
"gfx90a-insts")

diff  --git a/clang/lib/Basic/Targets/AMDGPU.cpp 
b/clang/lib/Basic/Targets/AMDGPU.cpp
index 72dfb07804dff..9b3a0b0f40edb 100644
--- a/clang/lib/Basic/Targets/AMDGPU.cpp
+++ b/clang/lib/Basic/Targets/AMDGPU.cpp
@@ -206,6 +206,7 @@ bool AMDGPUTargetInfo::initFeatureMap(
   Features["gfx10-insts"] = true;
   Features["gfx10-3-insts"] = true;
   Features["gfx11-insts"] = true;
+  Features["atomic-fadd-rtn-insts"] = true;
   break;
 case GK_GFX1036:
 case GK_GFX1035:
@@ -264,6 +265,7 @@ bool AMDGPUTargetInfo::initFeatureMap(
 case GK_GFX90A:
   Features["gfx90a-insts"] = true;
   Features["atomic-buffer-global-pk-add-f16-insts"] = true;
+  Features["atomic-fadd-rtn-insts"] = true;
   [[fallthrough]];
 case GK_GFX908:
   Features["dot3-insts"] = true;

diff  --git a/clang/test/CodeGenOpenCL/amdgpu-features.cl 
b/clang/test/CodeGenOpenCL/amdgpu-features.cl
index 4a4da6b270b9a..e000239cd03fe 100644
--- a/clang/test/CodeGenOpenCL/amdgpu-features.cl
+++ b/clang/test/CodeGenOpenCL/amdgpu-features.cl
@@ -72,9 +72,9 @@
 // GFX906: 
"target-features"="+16-bit-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
 // GFX908: 
"target-features"="+16-bit-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
 // GFX909: 
"target-features"="+16-bit-insts,+ci-insts,+dpp,+gfx8-insts,+gfx9-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
-// GFX90A: 
"target-features"="+16-bit-insts,+atomic-buffer-global-pk-add-f16-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+gfx90a-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
+// GFX90A: 
"target-features"="+16-bit-insts,+atomic-buffer-global-pk-add-f16-insts,+atomic-fadd-rtn-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+gfx90a-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
 // GFX90C: 
"target-features"="+16-bit-insts,+ci-insts,+dpp,+gfx8-insts,+gfx9-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
-// GFX940: 

[PATCH] D146840: [AMDGPU] Replace target feature for global fadd32

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added a comment.

No objection here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146840

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

[PATCH] D147070: Improve requirement clause limitation on non templated function

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik accepted this revision.
shafik added a comment.

LGTM




Comment at: clang/lib/Sema/SemaDecl.cpp:11877
 // member-declarator shall be present only if the declarator declares a
 // templated function ([dcl.fct]).
 if (Expr *TRC = NewFD->getTrailingRequiresClause()) {

The cross reference was fixed: https://github.com/cplusplus/draft/pull/6215


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

https://reviews.llvm.org/D147070

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

[clang] 9f15f1f - [analyzer] Teach scan-build how to pass -analyzer-config to xcodebuild.

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2023-03-28T14:34:28-07:00
New Revision: 9f15f1f0f3f58da8600c78c60032299f5ee4d6c5

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

LOG: [analyzer] Teach scan-build how to pass -analyzer-config to xcodebuild.

The scan-build tool assists various build systems with applying the Clang
static analyzer alongside compilation. It offers explicit integration with
Xcode's native build system aka `xcodebuild`; in this case it doesn't
substitute the compiler, but instead kindly asks xcodebuild to enable
the static analyzer, something that it already knows how to do.

Make sure scan-build's `-analyzer-config` flag (which translates to a
similar `clang -cc1 -analyzer-config` flag) is properly translated
to Xcode build system. This unbreaks a few related features such as
checker silencing.

No LIT tests because they'd require an Xcode installation on your system.

Added: 


Modified: 
clang/tools/scan-build/bin/scan-build

Removed: 




diff  --git a/clang/tools/scan-build/bin/scan-build 
b/clang/tools/scan-build/bin/scan-build
index 8cd525f054fdc..04734d9cfa9af 100755
--- a/clang/tools/scan-build/bin/scan-build
+++ b/clang/tools/scan-build/bin/scan-build
@@ -1037,7 +1037,8 @@ sub RunXcodebuild {
   if ($oldBehavior == 0) {
 my $OutputDir = $EnvVars->{"OUTPUT_DIR"};
 my $CLANG = $EnvVars->{"CLANG"};
-my $OtherFlags = $EnvVars->{"CCC_ANALYZER_ANALYSIS"};
+my $OtherFlags = $EnvVars->{"CCC_ANALYZER_ANALYSIS"} . " "
+   . $EnvVars->{"CCC_ANALYZER_CONFIG"};
 push @$Args,
 "RUN_CLANG_STATIC_ANALYZER=YES",
 "CLANG_ANALYZER_OUTPUT=plist-html",



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

[PATCH] D146840: [AMDGPU] Replace target feature for global fadd32

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added a comment.

Sounds good, thanks the review @rampitec


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146840

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

[PATCH] D147081: [clang-tidy] Fix issues in bugprone-assert-side-effect

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 509141.
PiotrZSL added a comment.

Fix auto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147081

Files:
  clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/assert-side-effect.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/assert-side-effect/assert.h
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect-constexpr.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp
@@ -1,47 +1,12 @@
-// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, {key: bugprone-assert-side-effect.IgnoredFunctions, value: 'MyClass::badButIgnoredFunc'}]}" -- -fexceptions
-
-//===--- assert definition block --===//
-int abort() { return 0; }
-
-#ifdef NDEBUG
-#define assert(x) 1
-#else
-#define assert(x)  \
-  if (!(x))\
-  (void)abort()
-#endif
-
-void print(...);
-#define assert2(e) (__builtin_expect(!(e), 0) ?\
-   print (#e, __FILE__, __LINE__) : (void)0)
-
-#ifdef NDEBUG
-#define my_assert(x) 1
-#else
-#define my_assert(x)   \
-  ((void)((x) ? 1 : abort()))
-#endif
-
-#ifdef NDEBUG
-#define not_my_assert(x) 1
-#else
-#define not_my_assert(x)   \
-  if (!(x))\
-  (void)abort()
-#endif
-
-#define real_assert(x) ((void)((x) ? 1 : abort()))
-#define wrap1(x) real_assert(x)
-#define wrap2(x) wrap1(x)
-#define convoluted_assert(x) wrap2(x)
-
-#define msvc_assert(expression) (void)(\
-(!!(expression)) ||\
-(abort(), 0)   \
-)
-
-
-//===--===//
+// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- \
+// RUN: -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, \
+// RUN:  {key: bugprone-assert-side-effect.CheckMethodCalls, value: true}, \
+// RUN:  {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, \
+// RUN:  {key: bugprone-assert-side-effect.IgnoredFunctions, value: 'functionToIgnore'}, \
+// RUN:  {key: bugprone-assert-side-effect.IgnoredMethods, value: 'MyClass::badButIgnoredFunc'}]}" \
+// RUN: -- -fexceptions -isystem %S/Inputs/assert-side-effect
+
+#include 
 
 bool badButIgnoredFunc(int a, int b) { return a * b > 0; }
 
@@ -69,6 +34,22 @@
   return true;
 }
 
+bool functionToIgnore() {
+  return true;
+}
+
+bool constFunction(bool input) __attribute__ ((const)) {
+  return !input;
+}
+
+bool pureFunction(bool input) __attribute__ ((pure)) {
+  return !input;
+}
+
+constexpr bool constexprFunction(bool input) {
+  return !input;
+}
+
 int main() {
 
   int X = 0;
@@ -126,5 +107,10 @@
   msvc_assert(mc2 = mc);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in msvc_assert() condition discarded in release builds
 
+  assert(functionToIgnore());
+  assert(constFunction(false));
+  assert(pureFunction(false));
+  assert(constexprFunction(false));
+
   return 0;
 }
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect-constexpr.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect-constexpr.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-assert-side-effect %t -- \
+// RUN: -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, \
+// RUN:  {key: bugprone-assert-side-effect.CheckMethodCalls, value: true}, \
+// RUN:  {key: bugprone-assert-side-effect.AssertMacros, value: 

[PATCH] D146840: [AMDGPU] Replace target feature for global fadd32

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

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

LGTM. Please wait for @b-sumner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146840

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

[PATCH] D146995: [clang-format][NFC] Refactor unit tests for "LambdaBodyIndentation: OuterScope" option.

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added a comment.

> You need to state a name and email for the commit.

Name: Jon Phillips
Email: jonap2...@gmail.com

Also, thanks for relating the child diff to this one. I couldn't see how to do 
it at first but have seen the option now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146995

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

[PATCH] D146887: [clang-tidy] Fix if-constexpr false-positive in readability-misleading-indentation

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 509136.
PiotrZSL added a comment.

Move tests to new file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146887

Files:
  clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/misleading-indentation-cpp17.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/misleading-indentation-cpp17.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/misleading-indentation-cpp17.cpp
@@ -0,0 +1,29 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s 
readability-misleading-indentation %t -- -- -fno-delayed-template-parsing
+
+namespace PR61435 {
+
+template
+constexpr auto lam_correct = []{
+  if constexpr (N == 1) {
+  } else {
+  }
+};
+
+template
+constexpr auto lam_incorrect = []{
+  if constexpr (N == 1) {
+  }
+   else {
+  }
+  // CHECK-MESSAGES: :[[@LINE-2]]:4: warning: different indentation for 'if' 
and corresponding 'else' [readability-misleading-indentation]
+};
+
+void test() {
+  lam_correct<1>();
+  lam_correct<2>();
+
+  lam_incorrect<1>();
+  lam_incorrect<2>();
+}
+
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -240,6 +240,10 @@
   magic numbers in type aliases such as ``using`` and ``typedef`` declarations 
if
   the new ``IgnoreTypeAliases`` option is set to true.
 
+- Fixed a false positive in :doc:`readability-misleading-indentation
+  ` check when warning 
would
+  be unnecessarily emitted for template dependent ``if constexpr``.
+
 - Fixed a false positive in :doc:`cppcoreguidelines-slicing
   ` check when warning would be
   emitted in constructor for virtual base class initialization.
Index: clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -104,7 +104,8 @@
 }
 
 void MisleadingIndentationCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(ifStmt(hasElse(stmt())).bind("if"), this);
+  Finder->addMatcher(
+  ifStmt(unless(hasThen(nullStmt())), hasElse(stmt())).bind("if"), this);
   Finder->addMatcher(
   compoundStmt(has(stmt(anyOf(ifStmt(), forStmt(), whileStmt()
   .bind("compound"),


Index: clang-tools-extra/test/clang-tidy/checkers/readability/misleading-indentation-cpp17.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/misleading-indentation-cpp17.cpp
@@ -0,0 +1,29 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s readability-misleading-indentation %t -- -- -fno-delayed-template-parsing
+
+namespace PR61435 {
+
+template
+constexpr auto lam_correct = []{
+  if constexpr (N == 1) {
+  } else {
+  }
+};
+
+template
+constexpr auto lam_incorrect = []{
+  if constexpr (N == 1) {
+  }
+   else {
+  }
+  // CHECK-MESSAGES: :[[@LINE-2]]:4: warning: different indentation for 'if' and corresponding 'else' [readability-misleading-indentation]
+};
+
+void test() {
+  lam_correct<1>();
+  lam_correct<2>();
+
+  lam_incorrect<1>();
+  lam_incorrect<2>();
+}
+
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -240,6 +240,10 @@
   magic numbers in type aliases such as ``using`` and ``typedef`` declarations if
   the new ``IgnoreTypeAliases`` option is set to true.
 
+- Fixed a false positive in :doc:`readability-misleading-indentation
+  ` check when warning would
+  be unnecessarily emitted for template dependent ``if constexpr``.
+
 - Fixed a false positive in :doc:`cppcoreguidelines-slicing
   ` check when warning would be
   emitted in constructor for virtual base class initialization.
Index: clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -104,7 +104,8 @@
 }
 
 void MisleadingIndentationCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(ifStmt(hasElse(stmt())).bind("if"), this);
+  Finder->addMatcher(
+  ifStmt(unless(hasThen(nullStmt())), hasElse(stmt())).bind("if"), this);
   Finder->addMatcher(
   compoundStmt(has(stmt(anyOf(ifStmt(), forStmt(), whileStmt()
   .bind("compound"),

[PATCH] D147081: [clang-tidy] Fix issues in bugprone-assert-side-effect

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp:66
+
+  const auto *FuncDecl = CExpr->getDirectCallee();
+  if (!FuncDecl || !FuncDecl->getDeclName().isIdentifier())

PiotrZSL wrote:
> Eugene.Zelenko wrote:
> > Please do not use `auto` if type is not spelled explicitly or iterator.
> That `auto` already was there, I just moved it... :)
So it's great reason to finally fix it :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147081

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

[PATCH] D147081: [clang-tidy] Fix issues in bugprone-assert-side-effect

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp:66
+
+  const auto *FuncDecl = CExpr->getDirectCallee();
+  if (!FuncDecl || !FuncDecl->getDeclName().isIdentifier())

Eugene.Zelenko wrote:
> Please do not use `auto` if type is not spelled explicitly or iterator.
That `auto` already was there, I just moved it... :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147081

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

[PATCH] D146459: [clang][PowerPC] Remove remaining Darwin support

[David Tenty via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2fe49ea0d07d: [clang][PowerPC] Remove remaining Darwin 
support (authored by daltenty).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D146459?vs=506735=509133#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146459

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/Driver/default-toolchain.c
  clang/test/Parser/altivec.c
  clang/test/Parser/cxx-altivec.cpp
  clang/test/Preprocessor/init-ppc.c
  clang/test/Sema/altivec-init.c
  clang/test/SemaCXX/cxx-altivec.cpp

Index: clang/test/SemaCXX/cxx-altivec.cpp
===
--- clang/test/SemaCXX/cxx-altivec.cpp
+++ clang/test/SemaCXX/cxx-altivec.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple=powerpc-apple-darwin8 -target-feature +altivec -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix -target-feature +altivec -fsyntax-only -verify %s
 
 struct Vector {
 	__vector float xyzw;
Index: clang/test/Sema/altivec-init.c
===
--- clang/test/Sema/altivec-init.c
+++ clang/test/Sema/altivec-init.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple=powerpc-apple-darwin8 -target-feature +altivec -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 %s -triple=powerpc-ibm-aix -target-feature +altivec -verify -pedantic -fsyntax-only
 
 typedef int v4 __attribute((vector_size(16)));
 typedef short v8 __attribute((vector_size(16)));
Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -975,202 +975,5 @@
 // PPC8548:#define __NO_LWSYNC__ 1
 // PPC8548:#define __SPE__ 1
 
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-apple-darwin8 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-DARWIN %s
-//
-// PPC-DARWIN:#define _ARCH_PPC 1
-// PPC-DARWIN:#define _BIG_ENDIAN 1
-// PPC-DARWIN:#define __BIGGEST_ALIGNMENT__ 16
-// PPC-DARWIN:#define __BIG_ENDIAN__ 1
-// PPC-DARWIN:#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
-// PPC-DARWIN:#define __CHAR16_TYPE__ unsigned short
-// PPC-DARWIN:#define __CHAR32_TYPE__ unsigned int
-// PPC-DARWIN:#define __CHAR_BIT__ 8
-// PPC-DARWIN:#define __DBL_DENORM_MIN__ 4.9406564584124654e-324
-// PPC-DARWIN:#define __DBL_DIG__ 15
-// PPC-DARWIN:#define __DBL_EPSILON__ 2.2204460492503131e-16
-// PPC-DARWIN:#define __DBL_HAS_DENORM__ 1
-// PPC-DARWIN:#define __DBL_HAS_INFINITY__ 1
-// PPC-DARWIN:#define __DBL_HAS_QUIET_NAN__ 1
-// PPC-DARWIN:#define __DBL_MANT_DIG__ 53
-// PPC-DARWIN:#define __DBL_MAX_10_EXP__ 308
-// PPC-DARWIN:#define __DBL_MAX_EXP__ 1024
-// PPC-DARWIN:#define __DBL_MAX__ 1.7976931348623157e+308
-// PPC-DARWIN:#define __DBL_MIN_10_EXP__ (-307)
-// PPC-DARWIN:#define __DBL_MIN_EXP__ (-1021)
-// PPC-DARWIN:#define __DBL_MIN__ 2.2250738585072014e-308
-// PPC-DARWIN:#define __DECIMAL_DIG__ __LDBL_DECIMAL_DIG__
-// PPC-DARWIN:#define __FLT_DENORM_MIN__ 1.40129846e-45F
-// PPC-DARWIN:#define __FLT_DIG__ 6
-// PPC-DARWIN:#define __FLT_EPSILON__ 1.19209290e-7F
-// PPC-DARWIN:#define __FLT_HAS_DENORM__ 1
-// PPC-DARWIN:#define __FLT_HAS_INFINITY__ 1
-// PPC-DARWIN:#define __FLT_HAS_QUIET_NAN__ 1
-// PPC-DARWIN:#define __FLT_MANT_DIG__ 24
-// PPC-DARWIN:#define __FLT_MAX_10_EXP__ 38
-// PPC-DARWIN:#define __FLT_MAX_EXP__ 128
-// PPC-DARWIN:#define __FLT_MAX__ 3.40282347e+38F
-// PPC-DARWIN:#define __FLT_MIN_10_EXP__ (-37)
-// PPC-DARWIN:#define __FLT_MIN_EXP__ (-125)
-// PPC-DARWIN:#define __FLT_MIN__ 1.17549435e-38F
-// PPC-DARWIN:#define __FLT_RADIX__ 2
-// PPC-DARWIN:#define __HAVE_BSWAP__ 1
-// PPC-DARWIN:#define __INT16_C_SUFFIX__
-// PPC-DARWIN:#define __INT16_FMTd__ "hd"
-// PPC-DARWIN:#define __INT16_FMTi__ "hi"
-// PPC-DARWIN:#define __INT16_MAX__ 32767
-// PPC-DARWIN:#define __INT16_TYPE__ short
-// PPC-DARWIN:#define __INT32_C_SUFFIX__
-// PPC-DARWIN:#define __INT32_FMTd__ "d"
-// PPC-DARWIN:#define __INT32_FMTi__ "i"
-// PPC-DARWIN:#define __INT32_MAX__ 2147483647
-// PPC-DARWIN:#define __INT32_TYPE__ int
-// PPC-DARWIN:#define __INT64_C_SUFFIX__ LL
-// PPC-DARWIN:#define __INT64_FMTd__ "lld"
-// PPC-DARWIN:#define __INT64_FMTi__ "lli"
-// PPC-DARWIN:#define __INT64_MAX__ 9223372036854775807LL
-// PPC-DARWIN:#define __INT64_TYPE__ long long int
-// PPC-DARWIN:#define __INT8_C_SUFFIX__
-// PPC-DARWIN:#define __INT8_FMTd__ "hhd"
-// PPC-DARWIN:#define __INT8_FMTi__ "hhi"
-// PPC-DARWIN:#define __INT8_MAX__ 127
-// PPC-DARWIN:#define __INT8_TYPE__ signed char
-// PPC-DARWIN:#define __INTMAX_C_SUFFIX__ LL
-// PPC-DARWIN:#define 

[clang] 2fe49ea - [clang][PowerPC] Remove remaining Darwin support

[David Tenty via cfe-commits]

Author: David Tenty
Date: 2023-03-28T17:12:49-04:00
New Revision: 2fe49ea0d07d503aedd0872bf0a66724552d4dcf

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

LOG: [clang][PowerPC] Remove remaining Darwin support

POWER Darwin support in the backend has been removed for some time: 
https://discourse.llvm.org/t/rfc-remove-darwin-support-from-power-backends
but Clang still has the TargetInfo and other remnants lying around.

This patch does some cleanup and removes those and other related frontend 
support still remaining. We adjust any tests using the triple to either remove
the test if unneeded or switch to another Power triple.

Reviewed By: MaskRay, nemanjai

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

Added: 


Modified: 
clang/lib/Basic/Targets.cpp
clang/lib/Basic/Targets/PPC.cpp
clang/lib/Basic/Targets/PPC.h
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/Driver/ToolChains/Darwin.cpp
clang/lib/Sema/SemaAttr.cpp
clang/test/Driver/default-toolchain.c
clang/test/Parser/altivec.c
clang/test/Parser/cxx-altivec.cpp
clang/test/Preprocessor/init-ppc.c
clang/test/Sema/altivec-init.c
clang/test/SemaCXX/cxx-altivec.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets.cpp b/clang/lib/Basic/Targets.cpp
index 7e687c119c1c4..b8932b1d56a20 100644
--- a/clang/lib/Basic/Targets.cpp
+++ b/clang/lib/Basic/Targets.cpp
@@ -347,8 +347,6 @@ TargetInfo *AllocateTarget(const llvm::Triple ,
 return new Le64TargetInfo(Triple, Opts);
 
   case llvm::Triple::ppc:
-if (Triple.isOSDarwin())
-  return new DarwinPPC32TargetInfo(Triple, Opts);
 switch (os) {
 case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
@@ -377,8 +375,6 @@ TargetInfo *AllocateTarget(const llvm::Triple ,
 }
 
   case llvm::Triple::ppc64:
-if (Triple.isOSDarwin())
-  return new DarwinPPC64TargetInfo(Triple, Opts);
 switch (os) {
 case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);

diff  --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index d46b1c55cf818..b27561729dfc4 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -336,9 +336,8 @@ void PPCTargetInfo::getTargetDefines(const LangOptions 
,
 Builder.defineMacro("__LONGDOUBLE64");
   }
 
-  // Define this for elfv2 (64-bit only) or 64-bit darwin.
-  if (ABI == "elfv2" ||
-  (getTriple().getOS() == llvm::Triple::Darwin && PointerWidth == 64))
+  // Define this for elfv2 (64-bit only).
+  if (ABI == "elfv2")
 Builder.defineMacro("__STRUCT_PARM_ALIGN__", "16");
 
   if (ArchDefs & ArchDefineName)

diff  --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
index 27fef6a2d5d47..07a8a3aefca7f 100644
--- a/clang/lib/Basic/Targets/PPC.h
+++ b/clang/lib/Basic/Targets/PPC.h
@@ -400,7 +400,7 @@ class LLVM_LIBRARY_VISIBILITY PPC32TargetInfo : public 
PPCTargetInfo {
   }
 
   BuiltinVaListKind getBuiltinVaListKind() const override {
-// This is the ELF definition, and is overridden by the Darwin sub-target
+// This is the ELF definition
 return TargetInfo::PowerABIBuiltinVaList;
   }
 };
@@ -481,33 +481,6 @@ class LLVM_LIBRARY_VISIBILITY PPC64TargetInfo : public 
PPCTargetInfo {
   }
 };
 
-class LLVM_LIBRARY_VISIBILITY DarwinPPC32TargetInfo
-: public DarwinTargetInfo {
-public:
-  DarwinPPC32TargetInfo(const llvm::Triple , const TargetOptions )
-  : DarwinTargetInfo(Triple, Opts) {
-HasAlignMac68kSupport = true;
-BoolWidth = BoolAlign = 32; // XXX support -mone-byte-bool?
-PtrDiffType = SignedInt; // for http://llvm.org/bugs/show_bug.cgi?id=15726
-LongLongAlign = 32;
-resetDataLayout("E-m:o-p:32:32-f64:32:64-n32", "_");
-  }
-
-  BuiltinVaListKind getBuiltinVaListKind() const override {
-return TargetInfo::CharPtrBuiltinVaList;
-  }
-};
-
-class LLVM_LIBRARY_VISIBILITY DarwinPPC64TargetInfo
-: public DarwinTargetInfo {
-public:
-  DarwinPPC64TargetInfo(const llvm::Triple , const TargetOptions )
-  : DarwinTargetInfo(Triple, Opts) {
-HasAlignMac68kSupport = true;
-resetDataLayout("E-m:o-i64:64-n32:64", "_");
-  }
-};
-
 class LLVM_LIBRARY_VISIBILITY AIXPPC32TargetInfo :
   public AIXTargetInfo {
 public:

diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index dd838c3746000..e8761879b7f05 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -467,7 +467,7 @@ unsigned TargetCodeGenInfo::getSizeOfUnwindException() 
const {
   // Verified for:
   //   x86-64 FreeBSD, Linux, Darwin
   //   x86-32 FreeBSD, Linux, Darwin
-  //   PowerPCLinux, Darwin
+  //   PowerPCLinux
   //   ARMDarwin (*not* EABI)
   //   AArch64Linux
   

[PATCH] D147081: [clang-tidy] Fix issues in bugprone-assert-side-effect

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp:66
+
+  const auto *FuncDecl = CExpr->getDirectCallee();
+  if (!FuncDecl || !FuncDecl->getDeclName().isIdentifier())

Please do not use `auto` if type is not spelled explicitly or iterator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147081

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

[PATCH] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

[Michael Buch via Phabricator via cfe-commits]

Michael137 added a comment.

In D145803#4220124 , @wolfgangp wrote:

> In D145803#4219894 , @probinson 
> wrote:
>
>> This is pretty different from the "always desugar to the canonical type" 
>> habit that has always been in place. Sony has done some downstream things to 
>> try to work around that in the past. @wolfgangp will remember it better than 
>> I do, but I think we make some effort to preserve the type-as-written. This 
>> goes in completely the other direction.
>
> The Sony solution does not rely on a user-specified attribute, but uniformly 
> preserves all typedefs, though only in template argument types. Similar to 
> this solution, it adds a second type to keep track of the preferred type, 
> though it does so in the TemplateArgument structure as a separate QualType. A 
> client can then either print the preferred type or the canonical type, 
> depending on a printing policy, which is controlled by an option. This leads 
> to a similar result in the resulting DWARF, i.e. the DW_AT_type node gets the 
> preferred type string. The preferred type can also be used for diagnostic 
> messages.

Neat, thanks for outlining your approach. IIUC, this basically just replicates 
the `PrintingPolicy::UsePreferredNames` when printing the names of template 
specializations. But it works for all typedefs, not just those annotated with 
the attribute.

@probinson Sounds like Sony's solution also changes the `DW_AT_type` to a 
non-canonical form. Do you still have concerns with the direction of this 
patch? Would it cause any problems for you downstream?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145803

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

[PATCH] D146840: [AMDGPU] Replace target feature for global fadd32

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 509127.
gandhi21299 added a comment.

- Adding tests for gfx90a and gfx940


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146840

Files:
  clang/include/clang/Basic/BuiltinsAMDGPU.def
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/CodeGenOpenCL/amdgpu-features.cl
  clang/test/CodeGenOpenCL/builtins-amdgcn-fp-atomics-gfx908-err.cl
  clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
  clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
  clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx940.cl

Index: clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx940.cl
===
--- clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx940.cl
+++ clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx940.cl
@@ -64,3 +64,9 @@
 void test_local_add_2f16_noret(__local half2 *addr, half2 x) {
   __builtin_amdgcn_ds_atomic_fadd_v2f16(addr, x);
 }
+
+// CHECK-LABEL: @test_global_add_f32
+// CHECK: call float @llvm.amdgcn.global.atomic.fadd.f32.p1.f32(ptr addrspace(1) %{{.*}}, float %{{.*}})
+void test_global_add_f32(float *rtn, global float *addr, float x) {
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f32(addr, x);
+}
Index: clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
===
--- clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
+++ clang/test/CodeGenOpenCL/builtins-fp-atomics-gfx90a.cl
@@ -115,3 +115,9 @@
   float *rtn;
   *rtn = __builtin_amdgcn_ds_atomic_fadd_f32(addr, x);
 }
+
+// CHECK-LABEL: @test_global_add_f32
+// CHECK: call float @llvm.amdgcn.global.atomic.fadd.f32.p1.f32(ptr addrspace(1) %{{.*}}, float %{{.*}})
+void test_global_add_f32(float *rtn, global float *addr, float x) {
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f32(addr, x);
+}
Index: clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
===
--- clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
+++ clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
@@ -43,3 +43,9 @@
 void test_s_wait_event_export_ready() {
   __builtin_amdgcn_s_wait_event_export_ready();
 }
+
+// CHECK-LABEL: @test_global_add_f32
+// CHECK: call float @llvm.amdgcn.global.atomic.fadd.f32.p1.f32(ptr addrspace(1) %{{.*}}, float %{{.*}})
+void test_global_add_f32(float *rtn, global float *addr, float x) {
+  *rtn = __builtin_amdgcn_global_atomic_fadd_f32(addr, x);
+}
Index: clang/test/CodeGenOpenCL/builtins-amdgcn-fp-atomics-gfx908-err.cl
===
--- clang/test/CodeGenOpenCL/builtins-amdgcn-fp-atomics-gfx908-err.cl
+++ clang/test/CodeGenOpenCL/builtins-amdgcn-fp-atomics-gfx908-err.cl
@@ -11,7 +11,7 @@
   float *fp_rtn;
   double *rtn;
   *half_rtn = __builtin_amdgcn_global_atomic_fadd_v2f16(addrh2, xh2); // expected-error{{'__builtin_amdgcn_global_atomic_fadd_v2f16' needs target feature atomic-buffer-global-pk-add-f16-insts}}
-  *fp_rtn = __builtin_amdgcn_global_atomic_fadd_f32(addr, x); // expected-error{{'__builtin_amdgcn_global_atomic_fadd_f32' needs target feature gfx90a-insts}}
+  *fp_rtn = __builtin_amdgcn_global_atomic_fadd_f32(addr, x); // expected-error{{'__builtin_amdgcn_global_atomic_fadd_f32' needs target feature atomic-fadd-rtn-insts}}
   *rtn = __builtin_amdgcn_global_atomic_fadd_f64(addr, x); // expected-error{{'__builtin_amdgcn_global_atomic_fadd_f64' needs target feature gfx90a-insts}}
   *rtn = __builtin_amdgcn_global_atomic_fmax_f64(addr, x); // expected-error{{'__builtin_amdgcn_global_atomic_fmax_f64' needs target feature gfx90a-insts}}
   *rtn = __builtin_amdgcn_global_atomic_fmin_f64(addr, x); // expected-error{{'__builtin_amdgcn_global_atomic_fmin_f64' needs target feature gfx90a-insts}}
Index: clang/test/CodeGenOpenCL/amdgpu-features.cl
===
--- clang/test/CodeGenOpenCL/amdgpu-features.cl
+++ clang/test/CodeGenOpenCL/amdgpu-features.cl
@@ -72,9 +72,9 @@
 // GFX906: "target-features"="+16-bit-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
 // GFX908: "target-features"="+16-bit-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
 // GFX909: "target-features"="+16-bit-insts,+ci-insts,+dpp,+gfx8-insts,+gfx9-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
-// GFX90A: "target-features"="+16-bit-insts,+atomic-buffer-global-pk-add-f16-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot3-insts,+dot4-insts,+dot5-insts,+dot6-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+gfx90a-insts,+mai-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
+// GFX90A: 

[PATCH] D146875: [clang-tidy] Fix example provided by add_new_check.py

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

I know that it would be better, best would be to remove that --fix-notes 
completely and enable those notes fixes with basic --fix, as it only create 
confusion.

Originally it were like this:
"Added an option to control whether to apply the fixes found in notes attached 
to clang tidy errors or not.
 Diagnostics may contain multiple notes each offering different ways to fix the 
issue, for that reason the default behaviour should be to not look at fixes 
found in notes.
 Instead offer up all the available fix-its in the output but don't try to 
apply the first one unless `-fix-notes` is supplied.
"
Commit: abbe9e227ed31e5dde9bb7567bb9f0dd047689c6 


But that is stupid, because there could be conflicts between those fixes.
And this were added by D59932 .

@njames93 What do you think, after all you broke this.
Option A) Change generator to attach fix to warning instead of note.
Option B) Revert your commit that introduced --fix-notes
Option C) Introduce option --dont-fix-notes, and enable notes fixes by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146875

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

[PATCH] D146101: [clang-format] Add DesignatedInitializerIndentWidth option.

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1665-1669
+  const auto DesignatedInitializerIndentWidth =
+  Style.DesignatedInitializerIndentWidth < 0
+  ? Style.ContinuationIndentWidth
+  : Style.DesignatedInitializerIndentWidth;
+  NewIndent = CurrentState.LastSpace + DesignatedInitializerIndentWidth;

HazardyKnusperkeks wrote:
> owenpan wrote:
> > jp4a50 wrote:
> > > owenpan wrote:
> > > > owenpan wrote:
> > > > > Using -1 to mean `ContinuationIndentWidth` is unnecessary and 
> > > > > somewhat confusing IMO.
> > > > Please disregard my comment above.
> > > Just to make sure we are on the same page, does this mean that you are 
> > > happy with the approach of using `-1` as a default value to indicate that 
> > > `ContinuationIndentWidth` should be used?
> > > 
> > > I did initially consider using `std::optional` and using empty 
> > > optional to indicate that `ContinuationIndentWidth` should be used but I 
> > > saw that `PPIndentWidth` was using `-1` to default to using `IndentWidth` 
> > > so I followed that precedent.
> > Yep! I would prefer the `optional`, but as you pointed out, we already got 
> > `PPIndentWidth`using `-1`.
> From the C++ side I totally agree. One could use `value_or()`, which would 
> make the code much more readable.
> And just because `PPIndentWidth` is using -1 is no reason to repeat that, we 
> could just as easily change `PPIndentWidht` to an optional.
> 
> But how would it look in yaml?
In YAML we wouldn't need to support empty optional being *explicitly* specified 
- it would just be the default.

So specifying `DesignatedInitializerIndentWidth: 4` would set the 
`std::optional` to `4` but if `DesignatedInitializerIndentWidth` was 
omitted from the YAML then the optional would simply not be set during parsing.

Of course, if we were to change `PPIndentWidth` to work that way too, it would 
technically be a breaking change because users may have *explicitly* specified 
`-1` in their YAML.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146101

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

[PATCH] D147081: [clang-tidy] Fix issues in bugprone-assert-side-effect

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

- Fixed problem when after changes in macros handling in Clang-tidy 14 cased 
issues from from this check be hidden when assert is defined in system headers. 
Now raising diagnostic for a place from where macro is called.
- Added support for const, pure, constexpr functions/methods. Now they 
considered safe.
- Split configuration options CheckFunctionCalls, IgnoredFunctions, to support 
enabling only checking of methods, or only of functions.

Fixes: #47605, #25569, #31821


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147081

Files:
  clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/assert-side-effect.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/assert-side-effect/assert.h
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect-constexpr.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp
@@ -1,47 +1,12 @@
-// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, {key: bugprone-assert-side-effect.IgnoredFunctions, value: 'MyClass::badButIgnoredFunc'}]}" -- -fexceptions
-
-//===--- assert definition block --===//
-int abort() { return 0; }
-
-#ifdef NDEBUG
-#define assert(x) 1
-#else
-#define assert(x)  \
-  if (!(x))\
-  (void)abort()
-#endif
-
-void print(...);
-#define assert2(e) (__builtin_expect(!(e), 0) ?\
-   print (#e, __FILE__, __LINE__) : (void)0)
-
-#ifdef NDEBUG
-#define my_assert(x) 1
-#else
-#define my_assert(x)   \
-  ((void)((x) ? 1 : abort()))
-#endif
-
-#ifdef NDEBUG
-#define not_my_assert(x) 1
-#else
-#define not_my_assert(x)   \
-  if (!(x))\
-  (void)abort()
-#endif
-
-#define real_assert(x) ((void)((x) ? 1 : abort()))
-#define wrap1(x) real_assert(x)
-#define wrap2(x) wrap1(x)
-#define convoluted_assert(x) wrap2(x)
-
-#define msvc_assert(expression) (void)(\
-(!!(expression)) ||\
-(abort(), 0)   \
-)
-
-
-//===--===//
+// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- \
+// RUN: -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, \
+// RUN:  {key: bugprone-assert-side-effect.CheckMethodCalls, value: true}, \
+// RUN:  {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, \
+// RUN:  {key: bugprone-assert-side-effect.IgnoredFunctions, value: 'functionToIgnore'}, \
+// RUN:  {key: bugprone-assert-side-effect.IgnoredMethods, value: 'MyClass::badButIgnoredFunc'}]}" \
+// RUN: -- -fexceptions -isystem %S/Inputs/assert-side-effect
+
+#include 
 
 bool badButIgnoredFunc(int a, int b) { return a * b > 0; }
 
@@ -69,6 +34,22 @@
   return true;
 }
 
+bool functionToIgnore() {
+  return true;
+}
+
+bool constFunction(bool input) __attribute__ ((const)) {
+  return !input;
+}
+
+bool pureFunction(bool input) __attribute__ ((pure)) {
+  return !input;
+}
+
+constexpr bool constexprFunction(bool input) {
+  return !input;
+}
+
 int main() {
 
   int X = 0;
@@ -126,5 +107,10 @@
   msvc_assert(mc2 = mc);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in msvc_assert() condition discarded in release builds
 
+  assert(functionToIgnore());
+  assert(constFunction(false));
+  assert(pureFunction(false));
+  assert(constexprFunction(false));
+
   return 0;
 }
Index: 

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

[Qiongsi Wu via Phabricator via cfe-commits]

qiongsiwu1 added a comment.

The new option is renamed to `-mxcoff-roptr` to avoid confusions if we are 
using it during a pure linking job on non-AIX platforms. The option name now 
implies that it applies to XCOFF only, so silently ignoring the option on other 
platforms is not that serious a problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144190

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

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

[Qiongsi Wu via Phabricator via cfe-commits]

qiongsiwu1 updated this revision to Diff 509118.
qiongsiwu1 edited the summary of this revision.
qiongsiwu1 added a comment.

Renaming the `-mroptr` option to `-mxcoff-roptr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144190

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/PowerPC/aix-roptr.c
  clang/test/Driver/ppc-roptr.c

Index: clang/test/Driver/ppc-roptr.c
===
--- /dev/null
+++ clang/test/Driver/ppc-roptr.c
@@ -0,0 +1,45 @@
+// RUN: %clang -### --target=powerpc-ibm-aix-xcoff -mxcoff-roptr %s 2>&1 | \
+// RUN: FileCheck %s --check-prefixes=ROPTR,LINK
+// RUN: %clang -### --target=powerpc-ibm-aix-xcoff -c -mxcoff-roptr %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=ROPTR
+// RUN: %clang -### --target=powerpc-ibm-aix-xcoff -mxcoff-roptr -mno-xcoff-roptr %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=NO_ROPTR
+
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mxcoff-roptr %s 2>&1 | \
+// RUN: FileCheck %s --check-prefixes=ROPTR,LINK
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -S -mxcoff-roptr %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=ROPTR
+// RUN: %clang -### --target=powerpc-ibm-aix-xcoff %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=NO_ROPTR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mxcoff-roptr -flto %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=LTO_ROPTR
+// RUN: touch %t.o
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mxcoff-roptr %t.o 2>&1 | \
+// RUN: FileCheck %s --check-prefix=LINK
+
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mxcoff-roptr \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mno-xcoff-roptr \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_NOROPTR_ERR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mxcoff-roptr -shared \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=SHARED_ERR
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mxcoff-roptr -flto \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mxcoff-roptr -flto -fno-data-sections \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mno-xcoff-roptr -flto -fno-data-sections \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=NO_DATA_SECTION_ERR
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mno-xcoff-roptr -flto \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_NOROPTR_ERR
+
+// ROPTR: "-mxcoff-roptr"
+// LINK: "-bforceimprw"
+// LTO_ROPTR: "-bplugin_opt:-mxcoff-roptr"
+// NO_ROPTR-NOT: "-mxcoff-roptr"
+// NO_ROPTR-NOT: "-bforceimprw"
+
+// DATA_SECTION_ERR: error: -mxcoff-roptr is supported only with -fdata-sections
+// NO_DATA_SECTION_ERR-NOT: error: -mxcoff-roptr is supported only with -fdata-sections
+// TARGET_ROPTR_ERR: error: unsupported option '-mxcoff-roptr' for target 'powerpc64le-unknown-linux-gnu'
+// TARGET_NOROPTR_ERR: error: unsupported option '-mno-xcoff-roptr' for target 'powerpc64le-unknown-linux-gnu'
+// SHARED_ERR: error: -mxcoff-roptr is not suppored with -shared
Index: clang/test/CodeGen/PowerPC/aix-roptr.c
===
--- /dev/null
+++ clang/test/CodeGen/PowerPC/aix-roptr.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix-xcoff -mxcoff-roptr -fdata-sections \
+// RUN: -S <%s | FileCheck %s --check-prefix=CHECK32
+// RUN: %clang_cc1 -triple=powerpc64-ibm-aix-xcoff -mxcoff-roptr -fdata-sections \
+// RUN: -S <%s | FileCheck %s --check-prefix=CHECK64
+// RUN: not %clang_cc1 -triple=powerpc-ibm-aix-xcoff -mxcoff-roptr \
+// RUN: -S <%s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: not %clang_cc1 -triple=powerpc64-ibm-aix-xcoff -mxcoff-roptr \
+// RUN: -S <%s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: not %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -mxcoff-roptr \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
+
+char c1 = 10;
+char c2 = 20;
+char* const c1_ptr = 
+// CHECK32: .csect c1_ptr[RO],2
+// CHECK32-NEXT:	.globl	c1_ptr[RO]
+// CHECK32-NEXT:	.align	2
+// CHECK32-NEXT:	.vbyte	4, c1[RW]
+
+// CHECK64: .csect c1_ptr[RO],3
+// CHECK64-NEXT:	.globl	c1_ptr[RO]
+// CHECK64-NEXT:	.align	3
+// CHECK64-NEXT:	.vbyte	8, c1[RW]
+
+// DATA_SECTION_ERR: error: -mxcoff-roptr is 

[clang-tools-extra] 4d4c0f9 - [clang-tidy] Add option to ignore capture default by reference in cppcoreguidelines-avoid-capture-default-when-capturing-this

[Carlos Galvez via cfe-commits]

Author: Carlos Galvez
Date: 2023-03-28T20:36:34Z
New Revision: 4d4c0f9734607bb0423593b060b8fa73c06fe3d3

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

LOG: [clang-tidy] Add option to ignore capture default by reference in 
cppcoreguidelines-avoid-capture-default-when-capturing-this

The rule exists primarily for when using capture default
by copy "[=]", since member variables will be captured by
reference, which is against developer expectations.

However when the capture default is by reference, then there
is no doubt: everything will be captured by reference. Add
an option to allow just that.

Note: Release Notes do not need update since this check
has been introduced in the current WIP release.

A ticket has been opened at the C++ Core Guidelines repo
to consider updating the rule such that this behavior
is the default one:
https://github.com/isocpp/CppCoreGuidelines/issues/2060

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

Added: 


Modified: 

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

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.h

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this.rst

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-default-when-capturing-this.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp
index 11ef35178765f..1489ca6c442b1 100644
--- 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp
@@ -18,6 +18,19 @@ using namespace clang::ast_matchers;
 
 namespace clang::tidy::cppcoreguidelines {
 
+AvoidCaptureDefaultWhenCapturingThisCheck::
+AvoidCaptureDefaultWhenCapturingThisCheck(StringRef Name,
+  ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IgnoreCaptureDefaultByReference(
+  Options.get("IgnoreCaptureDefaultByReference", false)) {}
+
+void AvoidCaptureDefaultWhenCapturingThisCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IgnoreCaptureDefaultByReference",
+IgnoreCaptureDefaultByReference);
+}
+
 void AvoidCaptureDefaultWhenCapturingThisCheck::registerMatchers(
 MatchFinder *Finder) {
   Finder->addMatcher(lambdaExpr(hasAnyCapture(capturesThis())).bind("lambda"),
@@ -74,24 +87,30 @@ static std::string createReplacementText(const LambdaExpr 
*Lambda) {
 
 void AvoidCaptureDefaultWhenCapturingThisCheck::check(
 const MatchFinder::MatchResult ) {
-  if (const auto *Lambda = Result.Nodes.getNodeAs("lambda")) {
-if (Lambda->getCaptureDefault() != LCD_None) {
-  bool IsThisImplicitlyCaptured = std::any_of(
-  Lambda->implicit_capture_begin(), Lambda->implicit_capture_end(),
-  [](const LambdaCapture ) { return Capture.capturesThis(); });
-  auto Diag = diag(Lambda->getCaptureDefaultLoc(),
-   "lambdas that %select{|implicitly }0capture 'this' "
-   "should not specify a capture default")
-  << IsThisImplicitlyCaptured;
-
-  std::string ReplacementText = createReplacementText(Lambda);
-  SourceLocation DefaultCaptureEnd =
-  findDefaultCaptureEnd(Lambda, *Result.Context);
-  Diag << FixItHint::CreateReplacement(
-  CharSourceRange::getCharRange(Lambda->getCaptureDefaultLoc(),
-DefaultCaptureEnd),
-  ReplacementText);
-}
+  const auto *Lambda = Result.Nodes.getNodeAs("lambda");
+  if (!Lambda)
+return;
+
+  if (IgnoreCaptureDefaultByReference &&
+  Lambda->getCaptureDefault() == LCD_ByRef)
+return;
+
+  if (Lambda->getCaptureDefault() != LCD_None) {
+bool IsThisImplicitlyCaptured = std::any_of(
+Lambda->implicit_capture_begin(), Lambda->implicit_capture_end(),
+[](const LambdaCapture ) { return Capture.capturesThis(); });
+auto Diag = diag(Lambda->getCaptureDefaultLoc(),
+ "lambdas that %select{|implicitly }0capture 'this' "
+ "should not specify a capture default")
+<< IsThisImplicitlyCaptured;
+
+std::string ReplacementText = createReplacementText(Lambda);
+SourceLocation DefaultCaptureEnd =
+findDefaultCaptureEnd(Lambda, *Result.Context);
+Diag << FixItHint::CreateReplacement(
+

[PATCH] D147062: [clang-tidy] Add option to ignore capture default by reference in cppcoreguidelines-avoid-capture-default-when-capturing-this

[Carlos Galvez 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 rG4d4c0f973460: [clang-tidy] Add option to ignore capture 
default by reference in… (authored by carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147062

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-default-when-capturing-this.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-default-when-capturing-this.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-default-when-capturing-this.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-default-when-capturing-this.cpp
@@ -1,4 +1,7 @@
-// RUN: %check_clang_tidy -std=c++11-or-later %s cppcoreguidelines-avoid-capture-default-when-capturing-this %t
+// RUN: %check_clang_tidy -std=c++11-or-later %s cppcoreguidelines-avoid-capture-default-when-capturing-this %t \
+// RUN: -check-suffixes=,DEFAULT
+// RUN: %check_clang_tidy -std=c++11-or-later %s cppcoreguidelines-avoid-capture-default-when-capturing-this %t \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-avoid-capture-default-when-capturing-this.IgnoreCaptureDefaultByReference, value: true}]}"
 
 struct Obj {
   void lambdas_that_warn_default_capture_copy() {
@@ -55,24 +58,24 @@
 int local2{};
 
 auto ref_explicit_this_capture = [&, this]() { };
-// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
-// CHECK-FIXES: auto ref_explicit_this_capture = [this]() { };
+// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:39: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
+// CHECK-FIXES-DEFAULT: auto ref_explicit_this_capture = [this]() { };
 
 auto ref_explicit_this_capture_local = [&, this]() { return (local+x) > 10; };
-// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
-// CHECK-FIXES: auto ref_explicit_this_capture_local = [, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:45: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
+// CHECK-FIXES-DEFAULT: auto ref_explicit_this_capture_local = [, this]() { return (local+x) > 10; };
 
 auto ref_implicit_this_capture = [&]() { return x > 10; };
-// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: lambdas that implicitly capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
-// CHECK-FIXES: auto ref_implicit_this_capture = [this]() { return x > 10; };
+// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:39: warning: lambdas that implicitly capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
+// CHECK-FIXES-DEFAULT: auto ref_implicit_this_capture = [this]() { return x > 10; };
 
 auto ref_implicit_this_capture_local = [&]() { return (local+x) > 10; };
-// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: lambdas that implicitly capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
-// CHECK-FIXES: auto ref_implicit_this_capture_local = [, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:45: warning: lambdas that implicitly capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
+// CHECK-FIXES-DEFAULT: auto ref_implicit_this_capture_local = [, this]() { return (local+x) > 10; };
 
 auto ref_implicit_this_capture_locals = [&]() { return (local+local2+x) > 10; };
-// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that implicitly capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
-// CHECK-FIXES: auto ref_implicit_this_capture_locals = [, , this]() { return (local+local2+x) > 10; };
+// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:46: warning: lambdas that implicitly capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-default-when-capturing-this]
+// CHECK-FIXES-DEFAULT: auto 

[PATCH] D146840: [AMDGPU] Replace target feature for global fadd32

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added a comment.

Can you please also add gfx90a and gfx940 tests?

Otherwise LGTM *if* @b-sumner has no objections.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146840

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

[PATCH] D146908: [clang][MinGW] Add asan DLL lib before other libs and objects

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: clang/test/Driver/mingw-sanitizers.c:2
+// RUN: touch %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent 
%/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s
+// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 
-lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-X86_64 
-DINPUT=%/t.a %s

alvinhochun wrote:
> mstorsjo wrote:
> > You're using `%/t.a` here while the file you touch is `%t.a` - I don't 
> > think I have seen `%/t` anywhere here before. I do see that lit seems to 
> > replace it though... Is this an intentional thing (what's the difference to 
> > `%t` though?) or is it a consistently copied typo?
> Ah yes, that is intentional. It's documented on 
> https://llvm.org/docs/CommandGuide/lit.html#substitutions to be "%t but \ is 
> replaced by /". The reason for this is that, the previous Windows pre-merge 
> check failed because when `clang -###` prints the command line it quotes and 
> backslash-escape the arguments., FileCheck cannot match the INPUT path with 
> double backslashes. Using forward slashes avoids this issue.
Oh, I see, thanks - TIL! That makes sense. I wonder why a grep for that didn't 
get almost any hits in the clang/test/Driver directory though - but maybe it's 
used more elsewhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146908

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:980-981
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)

The following input causes clang to crash:

```
$ clang -fno-integrated-as -c -x assembler-with-cpp /dev/null
clang: /android0/llvm-project/llvm/include/llvm/ADT/StringRef.h:597: 
llvm::StringRef llvm::StringRef::drop_front(size_t) const: Assertion `size() >= 
N && "Dropping more elements than exist"' failed.
```

Please fix that and add a test case for `/dev/null` (i.e. inputs for which no 
extension exists).


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

https://reviews.llvm.org/D145726

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

[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

How is this attribute going to handle a trampoline that performs a virtual 
dispatch from C++ call into Swift? In that case, the target is not known.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D147062: [clang-tidy] Add option to ignore capture default by reference in cppcoreguidelines-avoid-capture-default-when-capturing-this

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D147062#4228403 , @ccotter wrote:

> If https://github.com/isocpp/CppCoreGuidelines/issues/2060 is accepted to 
> only consider `[=]`, then I assume we'd want to change the default value of 
> IgnoreCaptureDefaultByReference to be true?  Thanks by the way for this!

Thanks for reviewing! If they do accept it (I'm not very optimistic) then I'd 
probably just remove this logic and exit early if the lambda capture is not 
`LCD_ByCopy`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147062

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

[PATCH] D146908: [clang][MinGW] Add asan DLL lib before other libs and objects

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun added inline comments.



Comment at: clang/test/Driver/mingw-sanitizers.c:2
+// RUN: touch %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent 
%/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s
+// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 
-lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-X86_64 
-DINPUT=%/t.a %s

mstorsjo wrote:
> You're using `%/t.a` here while the file you touch is `%t.a` - I don't think 
> I have seen `%/t` anywhere here before. I do see that lit seems to replace it 
> though... Is this an intentional thing (what's the difference to `%t` 
> though?) or is it a consistently copied typo?
Ah yes, that is intentional. It's documented on 
https://llvm.org/docs/CommandGuide/lit.html#substitutions to be "%t but \ is 
replaced by /". The reason for this is that, the previous Windows pre-merge 
check failed because when `clang -###` prints the command line it quotes and 
backslash-escape the arguments., FileCheck cannot match the INPUT path with 
double backslashes. Using forward slashes avoids this issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146908

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

[PATCH] D146887: [clang-tidy] Fix if-constexpr false-positive in readability-misleading-indentation

[Carlos Galvez via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/misleading-indentation.cpp:1
-// RUN: %check_clang_tidy %s readability-misleading-indentation %t -- -- 
-fno-delayed-template-parsing
+// RUN: %check_clang_tidy -std=c++17-or-later %s 
readability-misleading-indentation %t -- -- -fno-delayed-template-parsing
 

We should keep the default value of c++11-or-later, otherwise we are losing 
coverage on older versions.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/misleading-indentation.cpp:199
+
+namespace PR61435 {
+

I believe the most common way of solving this is creating another test file 
where you test the c++17-or-later support. Alternatively you'd need to wrap 
this code in a #if block, plus create another RUN line with a different 
`check-suffix`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146887

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Please update the commit message and comment about what binutils versions 
reject the construct.




Comment at: clang/test/Driver/gcc_forward.c:49
+// DEBUG-NOT: "-gdwarf-4"
\ No newline at end of file


No newline at end of file

Please fix


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

https://reviews.llvm.org/D145726

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

[PATCH] D147062: [clang-tidy] Add option to ignore capture default by reference in cppcoreguidelines-avoid-capture-default-when-capturing-this

[Chris Cotter via Phabricator via cfe-commits]

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

If https://github.com/isocpp/CppCoreGuidelines/issues/2060 is accepted to only 
consider `[=]`, then I assume we'd want to change the default value of 
IgnoreCaptureDefaultByReference to be true?  Thanks by the way for this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147062

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

[PATCH] D146844: [clang-format] Handle '_' in ud-suffix for IntegerLiteralSeparator

[Owen Pan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4d21868b0968: [clang-format] Handle _ in 
ud-suffix for IntegerLiteralSeparator (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146844

Files:
  clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
  clang/unittests/Format/IntegerLiteralSeparatorTest.cpp


Index: clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
===
--- clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
+++ clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
@@ -49,7 +49,21 @@
 
   verifyFormat("o0 = 0;\n"
"o1 = 07;\n"
-   "o5 = 012345",
+   "o5 = 012345;",
+   Style);
+
+  verifyFormat("bi = 0b1'i;\n"
+   "dif = 1'234if;\n"
+   "hil = 0xA'BCil;",
+   "bi = 0b1i;\n"
+   "dif = 1234if;\n"
+   "hil = 0xABCil;",
+   Style);
+
+  verifyFormat("d = 5'678_km;\n"
+   "h = 0xD'EF_u16;",
+   "d = 5678_km;\n"
+   "h = 0xDEF_u16;",
Style);
 }
 
Index: clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
===
--- clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
+++ clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
@@ -106,6 +106,12 @@
 (IsBase16 && SkipHex) || B == Base::Other) {
   continue;
 }
+if (Style.isCpp()) {
+  if (const auto Pos = Text.find_first_of("_i"); Pos != StringRef::npos) {
+Text = Text.substr(0, Pos);
+Length = Pos;
+  }
+}
 if ((IsBase10 && Text.find_last_of(".eEfFdDmM") != StringRef::npos) ||
 (IsBase16 && Text.find_last_of(".pP") != StringRef::npos)) {
   continue;
@@ -116,7 +122,7 @@
   continue;
 }
 const auto Start = Text[0] == '0' ? 2 : 0;
-auto End = Text.find_first_of("uUlLzZn");
+auto End = Text.find_first_of("uUlLzZn", Start);
 if (End == StringRef::npos)
   End = Length;
 if (Start > 0 || End < Length) {


Index: clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
===
--- clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
+++ clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
@@ -49,7 +49,21 @@
 
   verifyFormat("o0 = 0;\n"
"o1 = 07;\n"
-   "o5 = 012345",
+   "o5 = 012345;",
+   Style);
+
+  verifyFormat("bi = 0b1'i;\n"
+   "dif = 1'234if;\n"
+   "hil = 0xA'BCil;",
+   "bi = 0b1i;\n"
+   "dif = 1234if;\n"
+   "hil = 0xABCil;",
+   Style);
+
+  verifyFormat("d = 5'678_km;\n"
+   "h = 0xD'EF_u16;",
+   "d = 5678_km;\n"
+   "h = 0xDEF_u16;",
Style);
 }
 
Index: clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
===
--- clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
+++ clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
@@ -106,6 +106,12 @@
 (IsBase16 && SkipHex) || B == Base::Other) {
   continue;
 }
+if (Style.isCpp()) {
+  if (const auto Pos = Text.find_first_of("_i"); Pos != StringRef::npos) {
+Text = Text.substr(0, Pos);
+Length = Pos;
+  }
+}
 if ((IsBase10 && Text.find_last_of(".eEfFdDmM") != StringRef::npos) ||
 (IsBase16 && Text.find_last_of(".pP") != StringRef::npos)) {
   continue;
@@ -116,7 +122,7 @@
   continue;
 }
 const auto Start = Text[0] == '0' ? 2 : 0;
-auto End = Text.find_first_of("uUlLzZn");
+auto End = Text.find_first_of("uUlLzZn", Start);
 if (End == StringRef::npos)
   End = Length;
 if (Start > 0 || End < Length) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 4d21868 - [clang-format] Handle '_' in ud-suffix for IntegerLiteralSeparator

[Owen Pan via cfe-commits]

Author: Owen Pan
Date: 2023-03-28T13:13:02-07:00
New Revision: 4d21868b09681dffb9997f1a8d0c0dac12a226d3

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

LOG: [clang-format] Handle '_' in ud-suffix for IntegerLiteralSeparator

Also, handle imaginary numbers, i.e., those with suffixes starting
with an 'i'.

Fixes #61676.

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

Added: 


Modified: 
clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
clang/unittests/Format/IntegerLiteralSeparatorTest.cpp

Removed: 




diff  --git a/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp 
b/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
index 0eac3498d7215..35a8db0ce76e1 100644
--- a/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
+++ b/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
@@ -106,6 +106,12 @@ IntegerLiteralSeparatorFixer::process(const Environment 
,
 (IsBase16 && SkipHex) || B == Base::Other) {
   continue;
 }
+if (Style.isCpp()) {
+  if (const auto Pos = Text.find_first_of("_i"); Pos != StringRef::npos) {
+Text = Text.substr(0, Pos);
+Length = Pos;
+  }
+}
 if ((IsBase10 && Text.find_last_of(".eEfFdDmM") != StringRef::npos) ||
 (IsBase16 && Text.find_last_of(".pP") != StringRef::npos)) {
   continue;
@@ -116,7 +122,7 @@ IntegerLiteralSeparatorFixer::process(const Environment 
,
   continue;
 }
 const auto Start = Text[0] == '0' ? 2 : 0;
-auto End = Text.find_first_of("uUlLzZn");
+auto End = Text.find_first_of("uUlLzZn", Start);
 if (End == StringRef::npos)
   End = Length;
 if (Start > 0 || End < Length) {

diff  --git a/clang/unittests/Format/IntegerLiteralSeparatorTest.cpp 
b/clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
index 1c9ccebf44e99..eb0ca7e21ee5a 100644
--- a/clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
+++ b/clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
@@ -49,7 +49,21 @@ TEST_F(IntegerLiteralSeparatorTest, SingleQuoteAsSeparator) {
 
   verifyFormat("o0 = 0;\n"
"o1 = 07;\n"
-   "o5 = 012345",
+   "o5 = 012345;",
+   Style);
+
+  verifyFormat("bi = 0b1'i;\n"
+   "dif = 1'234if;\n"
+   "hil = 0xA'BCil;",
+   "bi = 0b1i;\n"
+   "dif = 1234if;\n"
+   "hil = 0xABCil;",
+   Style);
+
+  verifyFormat("d = 5'678_km;\n"
+   "h = 0xD'EF_u16;",
+   "d = 5678_km;\n"
+   "h = 0xDEF_u16;",
Style);
 }
 



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

[PATCH] D146844: [clang-format] Handle '_' in ud-suffix for IntegerLiteralSeparator

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D146844#4226558 , @MyDeveloperDay 
wrote:

> I'm not going to use it, but I like what you've done ;-)

Thanks! I won't use it either. :)


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

https://reviews.llvm.org/D146844

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

[PATCH] D147070: Improve requirement clause limitation on non templated function

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Thanks for the double check on the review!  I'm going to wait for the libcxx 
bot to come back (and for it to not be a commit at end of day!), but then will 
commit (unless there is new info).




Comment at: clang/test/SemaCXX/lambda-capture-type-deduction.cpp:52
+template
 void test_requires() {
 

cor3ntin wrote:
> erichkeane wrote:
> > When this isn't a template, all of the requires clauses below are 
> > ill-formed.  So made this a function template that hopefully @cor3ntin 
> > agrees matches his intent when he wrote this test.
> It does, thanks!
> It's weird that i didn't notice the bug when writing those tests - at the 
> same time i always felt this restriction to be a bit artificial.
Oh, absolutely.  Some of the limitations (on function defined inside of a 
function template that cannot be defined) are sensible in that they are 
otherwise unusable.  

I suspect the reasoning is that else we have to start mangling requires clauses 
even on non templates, AND it makes `extern "C"` not work for functions with 
requires clauses.


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

https://reviews.llvm.org/D147070

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

[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added subscribers: akhuang, bwyma, zturner.
aaron.ballman added a comment.

In D146595#4225702 , @aprantl wrote:

> In D146595#4225630 , @aaron.ballman 
> wrote:
>
>> It's less about other debug formats and more about user experience. My two 
>> big concerns are: 1) writing multiple attributes to do the same thing is 
>> somewhat user-hostile unless "the same thing" is actually different in some 
>> way users will care about, 2) we have a policy that user-facing attributes 
>> that do nothing are diagnosed as being ignored, so if we don't support 
>> Windows for this attribute, then compiling on that platform is going to 
>> generate a pile of "attribute ignored" warnings. If we can support the 
>> attribute with PDB as well, that solves both of my big concerns because it 
>> makes this attribute generally useful (which is what we aim for with 
>> attributes when possible).
>
> That's a valid concern. However, in its current form it would still get 
> lowered into LLVM IR, and LLVM may or may not silently ignore the attribute 
> when lowering to PDB. It could be argued that that's even worse than warning 
> about an ignored attribute. I just wanted to point this out.

That's the same for basically any LLVM IR metadata/attributes. We try our best 
from the FE to warn users if we know something is going to be ignored, but we 
don't have a diagnostic that is *guaranteed* to fire if the attribute doesn't 
do anything useful (then you run into all sorts of fun situations like: what if 
the attribute that does nothing is written on a function that is never called 
and so it's dead code stripped away?).

>> I don't want to sign y'all up for more work you can't easily do yourself, so 
>> I don't want to block on support for Windows unless it turns out to be 
>> completely trivial to support. But from a review POV, I'd like to hear back 
>> from someone who knows something about PDB to validate that the attribute 
>> doesn't require a breaking change for support there (like going from taking 
>> no args to requiring an arg for some reason, basically). If we don't think 
>> breaking changes should be needed, then I think documentation + diagnostics 
>> will be sufficient for me.
>>
>> I did have two questions though:
>> What happens when stepping into the attributed function through a function 
>> pointer?
>> Are there any special situations where a function cannot/should not be 
>> marked with the attribute (virtual functions, lambda, blocks)?
>
> @rnk Do you happen to know anything about how PDB handles these?

CC @akhuang @zturner @bwyma as other folks who might have ideas here as well 
(or know folks who do).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D147070: Improve requirement clause limitation on non templated function

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D147070#4228303 , @erichkeane 
wrote:

> Took Corentin's advice and tried to test the lambda condition.  THIS made me 
> discover that we didn't properly implement this for lambdas at all!  So this 
> patch NOW also implements the restriction for lambdas.
>
> @cor3ntin : Please double check on this for me?  I had to touch some lambda 
> tests you worked on.

Wow!
I think the patch still makes sense, I'm certainly glad we tested the lambda 
closure case!




Comment at: clang/test/SemaCXX/lambda-capture-type-deduction.cpp:52
+template
 void test_requires() {
 

erichkeane wrote:
> When this isn't a template, all of the requires clauses below are ill-formed. 
>  So made this a function template that hopefully @cor3ntin agrees matches his 
> intent when he wrote this test.
It does, thanks!
It's weird that i didn't notice the bug when writing those tests - at the same 
time i always felt this restriction to be a bit artificial.


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

https://reviews.llvm.org/D147070

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

[PATCH] D146188: [Clang][DOC] Add documentation in for __builtin_flt_rounds and __builtin_set_flt_rounds

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:3272
+3  - toward negative infinity
+4  - to nearest, ties away from zero
+The effect of passing some other value to ``__builtin_flt_rounds`` is

I suspect this won't end up being formatted as a list; you should do something 
like:

```
- ``0`` - toward zero
- ``1`` - to nearest, ties to even
...
```



Comment at: clang/docs/UsersManual.rst:1388
+by calling the `fesetround` function.
+
 Clang provides a number of ways to control floating point behavior, including

This isn't quite the right place for this.  Please put this new subsection 
after the described flags but before the "A note about..." subsections.

This file is RST, so you need to use double-backticks to format text in 
monospace.

This paragraph is just talking about the default mode which restricts access to 
the floating-point environment; let's add some text after it on how to get out 
of that default mode:

```
C provides two pragmas to allow code to dynamically modify the floating point 
environment:

- ``#pragma STDC FENV_ACCESS ON`` allows dynamic changes to the entire floating 
point environment.

- ``#pragma STDC FENV_ROUND FE_DYNAMIC`` allows dynamic changes to just the 
floating point rounding mode.  This may be more optimizable than ``FENV_ACCESS 
ON`` because the compiler can still ignore the possibility of floating-point 
exceptions by default.

Both of these can be used either at the start of a block scope, in which case 
they cover all code in that scope (unless they're turned off in a child scope), 
or at the top level in a file, in which case they cover all subsequent function 
bodies until they're turned off.  Note that it is undefined behavior to enter 
code that is *not* covered by one of these pragmas from code that *is* covered 
by one of these pragmas unless the floating point environment has been restored 
to its default state.  See the C standard for more information about these 
pragmas.

The command line option ``-frounding-math`` behaves as if the translation unit 
began with ``#pragma STDC FENV_ROUND FE_DYNAMIC``.  The command line option 
``-ffp-model=strict`` behaves as if the translation unit began with ``#pragma 
STDC FENV_ACCESS ON``.

Code that just wants to use a specific rounding mode for specific floating 
point operations can avoid most of the hazards of the dynamic floating point 
environment by using ``#pragma STDC FENV_ROUND`` with a value other than 
``FE_DYNAMIC``.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146188

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

[PATCH] D147037: [Clang][ICE] Corrected invalid invalid parameter index on some attributes with invalid indices applied to varargs functions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3764-3767
   if (!checkFunctionOrMethodParameterIndex(S, D, AL, 1, IdxExpr, Idx))
 return;
-
+  if (Idx.getASTIndex() >= getFunctionOrMethodNumParams(D))
+return;

aaron.ballman wrote:
> Did you look into fixing this within `checkFunctionOrMethodParameterIndex()` 
> instead? That way, all callers of the API get the correct behavior instead of 
> having to find individual attributes to check the logic (I believe there are 
> other attributes with the same underlying problem).
I would also expect any such issue to diagnose.  Also, isn't there an 
off-by-one error here in the case of variadic args?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147037

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

[PATCH] D147037: [Clang][ICE] Corrected invalid invalid parameter index on some attributes with invalid indices applied to varargs functions

[Aaron Ballman via Phabricator via cfe-commits]

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

Thanks for working on this fix! A few things:

- Please add a summary description to the patch, and be sure to include a 
reference to the github issue in the summary.
- All functional changes (e.g., not typo fixes, fixes to comments, or other NFC 
changes) need test coverage that demonstrates the issue has been resolved.
- All functional changes should have a release note in 
`clang/docs/ReleaseNotes.rst`




Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3764-3767
   if (!checkFunctionOrMethodParameterIndex(S, D, AL, 1, IdxExpr, Idx))
 return;
-
+  if (Idx.getASTIndex() >= getFunctionOrMethodNumParams(D))
+return;

Did you look into fixing this within `checkFunctionOrMethodParameterIndex()` 
instead? That way, all callers of the API get the correct behavior instead of 
having to find individual attributes to check the logic (I believe there are 
other attributes with the same underlying problem).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147037

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

[PATCH] D146875: [clang-tidy] Fix example provided by add_new_check.py

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/clang-tidy/add_new_check.py:278
   with io.open(filename, 'w', encoding='utf8', newline='\n') as f:
-f.write("""// RUN: %%check_clang_tidy %%s %(check_name_dashes)s %%t
+f.write("""// RUN: %%check_clang_tidy %%s %(check_name_dashes)s %%t -- 
--fix-notes
 

There is only 6 check tests in the clang-tidy folder that use `--fix-notes`, so 
I believe this is not quite standard. Thus I'm not convinced this should be 
part of the example test, which is supposed to be fairly basic. Would it be 
better to change the example check instead, so that the fix is not attached to 
the note? Example:


  diag(MatchedDecl->getLocation(), "function %%0 is insufficiently awesome, 
prepend it with 'awesome_'")
  << MatchedDecl
  << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "awesome_");


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146875

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

[PATCH] D147070: Improve requirement clause limitation on non templated function

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/test/Parser/cxx2b-lambdas.cpp:21
 auto L12 = [] consteval {};
-auto L13 = []() requires true {};
+auto L13 = []() requires true {}; // expected-error{{non-templated function 
cannot have a requires clause}}
 auto L14 = [] requires true() requires true {};

This still parses correctly, so I consider the error here to be 'correct' and 
still testing what we wanted.



Comment at: clang/test/SemaCXX/lambda-capture-type-deduction.cpp:52
+template
 void test_requires() {
 

When this isn't a template, all of the requires clauses below are ill-formed.  
So made this a function template that hopefully @cor3ntin agrees matches his 
intent when he wrote this test.


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

https://reviews.llvm.org/D147070

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

[PATCH] D147070: Improve requirement clause limitation on non templated function

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 509101.
erichkeane added a comment.

Took Corentin's advice and tried to test the lambda condition.  THIS made me 
discover that we didn't properly implement this for lambdas at all!  So this 
patch NOW also implements the restriction for lambdas.

@cor3ntin : Please double check on this for me?  I had to touch some lambda 
tests you worked on.


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

https://reviews.llvm.org/D147070

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/test/CXX/dcl.decl/dcl.decl.general/p4-20.cpp
  clang/test/Parser/cxx2b-lambdas.cpp
  clang/test/SemaCXX/lambda-capture-type-deduction.cpp

Index: clang/test/SemaCXX/lambda-capture-type-deduction.cpp
===
--- clang/test/SemaCXX/lambda-capture-type-deduction.cpp
+++ clang/test/SemaCXX/lambda-capture-type-deduction.cpp
@@ -48,6 +48,7 @@
   static_assert(noexcept([&] mutable noexcept(is_same) {}()));
 }
 
+template
 void test_requires() {
 
   int x;
@@ -77,6 +78,10 @@
   [x = 1]() mutable requires is_same {} ();
 }
 
+void use() {
+  test_requires();
+}
+
 void err() {
   int y, z;
   (void)[x = 1]
Index: clang/test/Parser/cxx2b-lambdas.cpp
===
--- clang/test/Parser/cxx2b-lambdas.cpp
+++ clang/test/Parser/cxx2b-lambdas.cpp
@@ -18,7 +18,7 @@
 auto L10 = [] noexcept { return true; };
 auto L11 = [] -> bool { return true; };
 auto L12 = [] consteval {};
-auto L13 = []() requires true {};
+auto L13 = []() requires true {}; // expected-error{{non-templated function cannot have a requires clause}}
 auto L14 = [] requires true() requires true {};
 auto L15 = [] requires true noexcept {};
 auto L16 = [] [[maybe_unused]]{};
Index: clang/test/CXX/dcl.decl/dcl.decl.general/p4-20.cpp
===
--- clang/test/CXX/dcl.decl/dcl.decl.general/p4-20.cpp
+++ clang/test/CXX/dcl.decl/dcl.decl.general/p4-20.cpp
@@ -23,3 +23,36 @@
 // expected-error@+1{{expected expression}}
 auto *p = new void(*)(char)
   requires true;
+
+namespace GH61748 {
+template
+struct S {
+  // expected-error@+1 {{non-templated function cannot have a requires clause}}
+  friend void declared_friend() requires(sizeof(T) > 1);
+  // OK, is a definition.
+  friend void defined_friend() requires(sizeof(T) > 1){}
+  // OK, is a member.
+  void member() requires(sizeof(T) > 1);
+};
+
+template
+void ContainingFunction() {
+  // expected-error@+1 {{non-templated function cannot have a requires clause}}
+  void bad() requires(sizeof(T) > 1);
+  // expected-error@+1 {{function definition is not allowed here}}
+  void still_bad() requires(sizeof(T) > 1) {}
+
+}
+
+void NonTemplContainingFunction() {
+  // expected-error@+1 {{non-templated function cannot have a requires clause}}
+  (void)[]() requires (sizeof(int)>1){};
+  // OK, a template.
+  auto X = [](auto) requires (sizeof(int)>1){};
+  // OK, a template.
+  auto Y = [](T t) requires (sizeof(int)>1){};
+
+  X(1);
+  Y(1);
+}
+}
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -1380,6 +1380,38 @@
 PushOnScopeChains(P, CurScope);
   }
 
+  // C++20: dcl.decl.general p4:
+  // The optional requires-clause ([temp.pre]) in an init-declarator or
+  // member-declarator shall be present only if the declarator declares a
+  // templated function ([dcl.fct]).
+  if (Expr *TRC = Method->getTrailingRequiresClause()) {
+// [temp.pre]/8:
+// An entity is templated if it is
+// - a template,
+// - an entity defined ([basic.def]) or created ([class.temporary]) in a
+// templated entity,
+// - a member of a templated entity,
+// - an enumerator for an enumeration that is a templated entity, or
+// - the closure type of a lambda-expression ([expr.prim.lambda.closure])
+// appearing in the declaration of a templated entity. [Note 6: A local
+// class, a local or block variable, or a friend function defined in a
+// templated entity is a templated entity.  — end note]
+//
+// A templated function is a function template or a function that is
+// templated. A templated class is a class template or a class that is
+// templated. A templated variable is a variable template or a variable
+// that is templated.
+
+// Note: we only have to check if this is defined in a template entity, OR
+// if we are a template, since the rest don't apply. The requires clause
+// applies to the call operator, which we already know is a member function,
+// AND defined.
+if (!Method->getDescribedFunctionTemplate() &&
+!Method->isTemplated()) {
+Diag(TRC->getBeginLoc(), diag::err_constrained_non_templated_function);
+}
+  }
+
   // Enter a new 

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

I'm trying to add the test case from: 
https://github.com/llvm/llvm-project/issues/45481#issuecomment-981028897, but 
lit test fails in `` not found.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147073

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

[PATCH] D145545: [clang][Interp] Fix local variable (destructor) management in loop bodies

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:209
+
+  return this->visitStmt(S);
+}

tbaeder wrote:
> aaron.ballman wrote:
> > It's a bit of a surprise that we visit the entire body of the compound 
> > statement before we visit the compound statement itself. I was thinking the 
> > compound statement could potentially have prologue work it needs to do, but 
> > now I'm second-guessing that. But this design still feels a little bit 
> > off... it's basically doing a post-order traversal just for this one node, 
> > and I wonder if we want something more general like a `preVisitFoo()` 
> > followed by `visitFoo()` followed by `postVisitFoo()` that applies to any 
> > AST node.
> I think you're overthinking this, this just for the `!isa(S)` 
> case (which IIRC happens at least for comma operator bin ops?)
Oh! Yeah, I totally overthought that. I got tripped up by this accepting things 
other than a `CompoundStmt` because of the function name. Sorry for the noise!



Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:324-328
+  LocalScope Scope(this);
+  if (!this->visitUnscopedCompoundStmt(Body))
 return false;
+
+  Scope.emitDestructors();

tbaeder wrote:
> aaron.ballman wrote:
> > `AutoScope` and some curly braces to delimit the scope object lifetime?
> The problem is that we want to emit the destructors before the jump, but not 
> destroy the scope. That should happen after the end label, when the loop is 
> finished altogether so an `AutoScope` doesn't work.
Ahh yeah, that's a good point... but this still worries me a bit because of how 
easy it is to misuse the scope after emitting the destructors. Notionally, we 
want two scopes, right? One scope for the loop construct guts and a second 
(inner) scope for the loop body. That's how the construct is modeled in the 
standard: http://eel.is/c++draft/stmt.while#2


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

https://reviews.llvm.org/D145545

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

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 509098.
zequanwu added a comment.

- Handle invalid start location.
- Add a test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147073

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/invalid_location.cpp


Index: clang/test/CoverageMapping/invalid_location.cpp
===
--- /dev/null
+++ clang/test/CoverageMapping/invalid_location.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false 
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
-emit-llvm-only -std=c++20 -triple %itanium_abi_triple %s | FileCheck %s
+
+namespace std { template  class initializer_list {}; }
+
+template  struct T {
+  T(std::initializer_list, int = int());
+  bool b;
+};
+
+template  struct S1 {
+  static void foo() {
+class C;
+0 ? T{} : T{};
+0 ? 1 : 2;
+  }
+};
+
+void bar() {
+  S1::foo();
+}
+
+
+// CHECK:  _ZN2S1IiE3fooEv:
+// CHECK-NEXT:   File 0, 11:21 -> 15:4 = #0
+// CHECK-NEXT:   File 0, 13:5 -> 13:6 = #0
+// CHECK-NEXT:   Branch,File 0, 13:5 -> 13:6 = 0, 0
+// CHECK-NEXT:   Gap,File 0, 13:8 -> 13:9 = #1
+// FIXME: It's supposed to have two different counters for true and false
+//branches at line 13 as it has for line 14, shown below. It's missing
+//now because 'T{}' doesn't have a valid end location for now.
+// CHECK-NETX:   File 0, 13:9 -> 13:14 = #3
+// CHECK-NETX:   File 0, 13:18 -> 13:23 = (#0 - #3)
+
+// CHECK-NEXT:   File 0, 14:5 -> 14:6 = #0
+// CHECK-NEXT:   Branch,File 0, 14:5 -> 14:6 = 0, 0
+// CHECK-NEXT:   Gap,File 0, 14:8 -> 14:9 = #2
+// CHECK-NEXT:   File 0, 14:9 -> 14:10 = #2
+// CHECK-NEXT:   File 0, 14:13 -> 14:14 = (#0 - #2)
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -602,6 +602,13 @@
   MostRecentLocation = *StartLoc;
 }
 
+// If the either location is invalid, set it to std::nullopt to avoid
+// letting users of RegionStack think that region has a valid start/end
+// location.
+if (StartLoc && StartLoc->isInvalid())
+  StartLoc = std::nullopt;
+if (EndLoc && EndLoc->isInvalid())
+  EndLoc = std::nullopt;
 RegionStack.emplace_back(Count, FalseCount, StartLoc, EndLoc);
 
 return RegionStack.size() - 1;
@@ -624,7 +631,8 @@
 assert(RegionStack.size() >= ParentIndex && "parent not in stack");
 while (RegionStack.size() > ParentIndex) {
   SourceMappingRegion  = RegionStack.back();
-  if (Region.hasStartLoc()) {
+  if (Region.hasStartLoc() &&
+  (Region.hasEndLoc() || RegionStack[ParentIndex].hasEndLoc())) {
 SourceLocation StartLoc = Region.getBeginLoc();
 SourceLocation EndLoc = Region.hasEndLoc()
 ? Region.getEndLoc()
@@ -691,7 +699,7 @@
 assert(SM.isWrittenInSameFile(Region.getBeginLoc(), EndLoc));
 assert(SpellingRegion(SM, Region).isInSourceOrder());
 SourceRegions.push_back(Region);
-}
+  }
   RegionStack.pop_back();
 }
   }
@@ -716,7 +724,8 @@
 
 // The statement may be spanned by an expansion. Make sure we handle a file
 // exit out of this expansion before moving to the next statement.
-if (SM.isBeforeInTranslationUnit(StartLoc, S->getBeginLoc()))
+if (StartLoc.isValid() &&
+SM.isBeforeInTranslationUnit(StartLoc, S->getBeginLoc()))
   MostRecentLocation = EndLoc;
 
 return ExitCount;
@@ -891,6 +900,8 @@
   /// Find a valid gap range between \p AfterLoc and \p BeforeLoc.
   std::optional findGapAreaBetween(SourceLocation AfterLoc,
 SourceLocation BeforeLoc) {
+if (AfterLoc.isInvalid() || BeforeLoc.isInvalid())
+  return std::nullopt;
 // If AfterLoc is in function-like macro, use the right parenthesis
 // location.
 if (AfterLoc.isMacroID()) {


Index: clang/test/CoverageMapping/invalid_location.cpp
===
--- /dev/null
+++ clang/test/CoverageMapping/invalid_location.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -std=c++20 -triple %itanium_abi_triple %s | FileCheck %s
+
+namespace std { template  class initializer_list {}; }
+
+template  struct T {
+  T(std::initializer_list, int = int());
+  bool b;
+};
+
+template  struct S1 {
+  static void foo() {
+class C;
+0 ? T{} : T{};
+0 ? 1 : 2;
+  }
+};
+
+void bar() {
+  S1::foo();
+}
+
+
+// CHECK:  _ZN2S1IiE3fooEv:
+// CHECK-NEXT:   File 0, 11:21 -> 15:4 = #0
+// CHECK-NEXT:   File 0, 13:5 -> 13:6 = #0
+// CHECK-NEXT:   Branch,File 

[PATCH] D146995: [clang-format][NFC] Refactor unit tests for "LambdaBodyIndentation: OuterScope" option.

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

HazardyKnusperkeks added a comment.

In D146995#4227532 , @jp4a50 wrote:

> If you guys are happy with this, could you please merge it for me?

You need to state a name and email for the commit.

> Edit: Also could you please advise about the failing CI test? It doesn't look 
> like it's anything to do with my changes but do let me know if that's not the 
> case.

If you have checked your code does compile and run on your side and the 
reported error is not in clang-format you can just ignore it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146995

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

[PATCH] D146908: [clang][MinGW] Add asan DLL lib before other libs and objects

[Martin Storsjö via Phabricator via cfe-commits]

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

LGTM overall.

I wondered if the modified test runs correctly on Windows (any test that 
touches paths is prone to break) but I see that it seems to have run 
successfully on both Windows and Linux in the premerge CI, so that's at least 
good. One minor question for the test.




Comment at: clang/test/Driver/mingw-sanitizers.c:2
+// RUN: touch %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent 
%/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s
+// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 
-lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-X86_64 
-DINPUT=%/t.a %s

You're using `%/t.a` here while the file you touch is `%t.a` - I don't think I 
have seen `%/t` anywhere here before. I do see that lit seems to replace it 
though... Is this an intentional thing (what's the difference to `%t` though?) 
or is it a consistently copied typo?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146908

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

[PATCH] D146408: [clang][Interp] Start supporting complex types

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I don't think the `_BitInt` test case should block this patch if that turns out 
to cause problems.




Comment at: clang/lib/AST/Interp/EvalEmitter.cpp:212-214
+});
+return false;
+  } else if (ElemTy->isFloatingType()) {

The switch is fully covered with an unreachable, and the outer level returns 
`false` anyway.



Comment at: clang/lib/AST/Interp/PrimType.h:108
+switch (Expr) {
\
+  TYPE_SWITCH_CASE(PT_Sint8, B)
\
+  TYPE_SWITCH_CASE(PT_Uint8, B)
\

Oh boy, this is going to be interesting, isn't it?
```
consteval _Complex _BitInt(12) UhOh() { return (_Complex _BitInt(12))1; }
consteval _Complex _BitInt(18) Why() { return (_Complex _BitInt(18))1; }

static_assert(UhOh() + Why() == 2);
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146408

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

[PATCH] D146101: [clang-format] Add DesignatedInitializerIndentWidth option.

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

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1665-1669
+  const auto DesignatedInitializerIndentWidth =
+  Style.DesignatedInitializerIndentWidth < 0
+  ? Style.ContinuationIndentWidth
+  : Style.DesignatedInitializerIndentWidth;
+  NewIndent = CurrentState.LastSpace + DesignatedInitializerIndentWidth;

owenpan wrote:
> jp4a50 wrote:
> > owenpan wrote:
> > > owenpan wrote:
> > > > Using -1 to mean `ContinuationIndentWidth` is unnecessary and somewhat 
> > > > confusing IMO.
> > > Please disregard my comment above.
> > Just to make sure we are on the same page, does this mean that you are 
> > happy with the approach of using `-1` as a default value to indicate that 
> > `ContinuationIndentWidth` should be used?
> > 
> > I did initially consider using `std::optional` and using empty 
> > optional to indicate that `ContinuationIndentWidth` should be used but I 
> > saw that `PPIndentWidth` was using `-1` to default to using `IndentWidth` 
> > so I followed that precedent.
> Yep! I would prefer the `optional`, but as you pointed out, we already got 
> `PPIndentWidth`using `-1`.
From the C++ side I totally agree. One could use `value_or()`, which would make 
the code much more readable.
And just because `PPIndentWidth` is using -1 is no reason to repeat that, we 
could just as easily change `PPIndentWidht` to an optional.

But how would it look in yaml?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146101

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

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

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

HazardyKnusperkeks added a comment.

Looks good to me, not giving my formal approval since the ongoing discussion 
about the style guide.




Comment at: clang/include/clang/Format/Format.h:823
+  /// \endcode
+  /// \version 16.0
+  bool AlwaysBreakBeforeFunctionParameters;

Since when do we add the `.0`?



Comment at: clang/lib/Format/TokenAnnotator.cpp:4742-4748
+  if (Left.is(tok::l_paren) && Style.AlwaysBreakBeforeFunctionParameters &&
+  !Right.is(tok::r_paren) && Left.Previous) {
+  const FormatToken  = *Left.Previous;
+  if (TwoPrevious.is(TT_FunctionDeclarationName)) {
+  return true;
+  }
+  }

jrmolin wrote:
> HazardyKnusperkeks wrote:
> > That I meant with merge.
> Ah, understood. I generally don't collapse `if`s, so I didn't know how much 
> you wanted. It seems like ... all of it.
I think now you need to format it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

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

[PATCH] D146926: [clang-format] Add option to decorate reflowed block comments

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

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/BreakableToken.cpp:406
   Decoration = "* ";
-  if (Lines.size() == 1 && !FirstInLine) {
+  if ((Lines.size() == 1 && !FirstInLine) || !Style.DecorateReflowedComments) {
 // Comments for which FirstInLine is false can start on arbitrary column,

So if set to `false` it will never add the decoration. But when set to `true` 
it still will not, if it's the first line.

This is not what I'd expect reading the documentation.

If you want to pursue this I think you should go for an enum with `Never`, 
`Always`, and `` (what we currently have).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146926

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added a comment.

In D145726#4228216 , @nickdesaulniers 
wrote:

> @garvitgupta08 what's your email address so that I may attribute your 
> authorship correctly when committing on your behalf?

quic_garvg...@quicinc.com. Thanks for committing!.


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

https://reviews.llvm.org/D145726

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

@garvitgupta08 what's your email address so that I may attribute your 
authorship correctly when committing on your behalf?


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

https://reviews.llvm.org/D145726

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

[PATCH] D147070: Improve requirement clause limitation on non templated function

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:11886-11887
+  // - an enumerator for an enumeration that is a templated entity, or
+  // - the closure type of a lambda-expression ([expr.prim.lambda.closure])
+  // appearing in the declaration of a templated entity. [Note 6: A local
+  // class, a local or block variable, or a friend function defined in a

cor3ntin wrote:
> I think we have no test for this case, might be worth adding.
Interestingly, we permit that, but do NOT diagnose the lambda with a requires 
clause in a non-template function.  Hrm


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

https://reviews.llvm.org/D147070

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

[PATCH] D147070: Improve requirement clause limitation on non templated function

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

LGTM except the arguably missing tests.




Comment at: clang/lib/Sema/SemaDecl.cpp:11886-11887
+  // - an enumerator for an enumeration that is a templated entity, or
+  // - the closure type of a lambda-expression ([expr.prim.lambda.closure])
+  // appearing in the declaration of a templated entity. [Note 6: A local
+  // class, a local or block variable, or a friend function defined in a

I think we have no test for this case, might be worth adding.


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

https://reviews.llvm.org/D147070

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

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

[Qiongsi Wu via Phabricator via cfe-commits]

qiongsiwu1 updated this revision to Diff 509091.
qiongsiwu1 added a comment.

Simplify LTO data section error check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144190

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/PowerPC/aix-roptr.c
  clang/test/Driver/ppc-roptr.c

Index: clang/test/Driver/ppc-roptr.c
===
--- /dev/null
+++ clang/test/Driver/ppc-roptr.c
@@ -0,0 +1,45 @@
+// RUN: %clang -### --target=powerpc-ibm-aix-xcoff -mroptr %s 2>&1 | \
+// RUN: FileCheck %s --check-prefixes=ROPTR,LINK
+// RUN: %clang -### --target=powerpc-ibm-aix-xcoff -c -mroptr %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=ROPTR
+// RUN: %clang -### --target=powerpc-ibm-aix-xcoff -mroptr -mno-roptr %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=NO_ROPTR
+
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mroptr %s 2>&1 | \
+// RUN: FileCheck %s --check-prefixes=ROPTR,LINK
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -S -mroptr %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=ROPTR
+// RUN: %clang -### --target=powerpc-ibm-aix-xcoff %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=NO_ROPTR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mroptr -flto %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=LTO_ROPTR
+// RUN: touch %t.o
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mroptr %t.o 2>&1 | \
+// RUN: FileCheck %s --check-prefix=LINK
+
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mroptr \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mno-roptr \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_NOROPTR_ERR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mroptr -shared \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=SHARED_ERR
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mroptr -flto \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mroptr -flto -fno-data-sections \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: %clang -### --target=powerpc64-ibm-aix-xcoff -mno-roptr -flto -fno-data-sections \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=NO_DATA_SECTION_ERR
+// RUN: %clang -### --target=powerpc64le-unknown-linux-gnu -mno-roptr -flto \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_NOROPTR_ERR
+
+// ROPTR: "-mroptr"
+// LINK: "-bforceimprw"
+// LTO_ROPTR: "-bplugin_opt:-mroptr"
+// NO_ROPTR-NOT: "-mroptr"
+// NO_ROPTR-NOT: "-bforceimprw"
+
+// DATA_SECTION_ERR: error: -mroptr is supported only with -fdata-sections
+// NO_DATA_SECTION_ERR-NOT: error: -mroptr is supported only with -fdata-sections
+// TARGET_ROPTR_ERR: error: unsupported option '-mroptr' for target 'powerpc64le-unknown-linux-gnu'
+// TARGET_NOROPTR_ERR: error: unsupported option '-mno-roptr' for target 'powerpc64le-unknown-linux-gnu'
+// SHARED_ERR: error: -mroptr is not suppored with -shared
Index: clang/test/CodeGen/PowerPC/aix-roptr.c
===
--- /dev/null
+++ clang/test/CodeGen/PowerPC/aix-roptr.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix-xcoff -mroptr -fdata-sections \
+// RUN: -S <%s | FileCheck %s --check-prefix=CHECK32
+// RUN: %clang_cc1 -triple=powerpc64-ibm-aix-xcoff -mroptr -fdata-sections \
+// RUN: -S <%s | FileCheck %s --check-prefix=CHECK64
+// RUN: not %clang_cc1 -triple=powerpc-ibm-aix-xcoff -mroptr \
+// RUN: -S <%s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: not %clang_cc1 -triple=powerpc64-ibm-aix-xcoff -mroptr \
+// RUN: -S <%s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: not %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -mroptr \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
+
+char c1 = 10;
+char c2 = 20;
+char* const c1_ptr = 
+// CHECK32: .csect c1_ptr[RO],2
+// CHECK32-NEXT:	.globl	c1_ptr[RO]
+// CHECK32-NEXT:	.align	2
+// CHECK32-NEXT:	.vbyte	4, c1[RW]
+
+// CHECK64: .csect c1_ptr[RO],3
+// CHECK64-NEXT:	.globl	c1_ptr[RO]
+// CHECK64-NEXT:	.align	3
+// CHECK64-NEXT:	.vbyte	8, c1[RW]
+
+// DATA_SECTION_ERR: error: -mroptr is supported only with -fdata-sections
+// TARGET_ROPTR_ERR: error: unsupported option '-mroptr' for target 'powerpc64le-unknown-linux-gnu'
+// TARGET_NOROPTR_ERR: error: unsupported option '-mno-roptr' for target 

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 509090.
jhuber6 added a comment.

Changing to use the `gpu-none-llvm` subfolder name that @sivachandra 
recommended. Also adding a `--sysroot` argument to show that this include path 
shows up first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/gpu-libc-headers.c


Index: clang/test/Driver/gpu-libc-headers.c
===
--- /dev/null
+++ clang/test/Driver/gpu-libc-headers.c
@@ -0,0 +1,22 @@
+// REQUIRES: nvptx-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp 
--sysroot=./ \
+// RUN: -fopenmp-targets=amdgcn-amd-amdhsa 
-Xopenmp-target=amdgcn-amd-amdhsa --offload-arch=gfx908  \
+// RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-HEADERS
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp 
--sysroot=./ \
+// RUN: -fopenmp-targets=nvptx64-nvidia-cuda 
-Xopenmp-target=nvptx64-nvidia-cuda --offload-arch=sm_70  \
+// RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-HEADERS
+// RUN:   %clang -### --target=nvptx64-nvidia-cuda -march=sm_70 -nogpulib 
--sysroot=./ %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-HEADERS
+// RUN:   %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx1030 -nogpulib 
--sysroot=./ %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-HEADERS
+// CHECK-HEADERS: "-cc1"{{.*}}"-c-isystem" 
"{{.*}}include/gpu-none-llvm"{{.*}}"-isysroot" "./"
+
+// RUN:   %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx1030 -nogpulib \
+// RUN: -nogpuinc %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-HEADERS-DISABLED
+// RUN:   %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx1030 -nogpulib \
+// RUN: -nostdinc %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-HEADERS-DISABLED
+// RUN:   %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx1030 -nogpulib \
+// RUN: -nobuiltininc %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-HEADERS-DISABLED
+// CHECK-HEADERS-DISABLED-NOT: "-cc1"{{.*}}"-c-isystem" 
"{{.*}}include/gpu-none-llvm"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1193,6 +1193,23 @@
   if (JA.isOffloading(Action::OFK_HIP))
 getToolChain().AddHIPIncludeArgs(Args, CmdArgs);
 
+  // If we are compiling for a GPU target we want to override the system 
headers
+  // with ones created by the 'libc' project if present.
+  if (!Args.hasArg(options::OPT_nostdinc) &&
+  !Args.hasArg(options::OPT_nogpuinc) &&
+  !Args.hasArg(options::OPT_nobuiltininc) &&
+  (getToolChain().getTriple().isNVPTX() ||
+   getToolChain().getTriple().isAMDGCN())) {
+
+  // Add include/gpu-none-libc/* to our system include path. This lets us 
use
+  // GPU-specific system headers first.
+  SmallString<128> P(llvm::sys::path::parent_path(D.InstalledDir));
+  llvm::sys::path::append(P, "include");
+  llvm::sys::path::append(P, "gpu-none-llvm");
+  CmdArgs.push_back("-c-isystem");
+  CmdArgs.push_back(Args.MakeArgString(P));
+  }
+
   // If we are offloading to a target via OpenMP we need to include the
   // openmp_wrappers folder which contains alternative system headers.
   if (JA.isDeviceOffloading(Action::OFK_OpenMP) &&


Index: clang/test/Driver/gpu-libc-headers.c
===
--- /dev/null
+++ clang/test/Driver/gpu-libc-headers.c
@@ -0,0 +1,22 @@
+// REQUIRES: nvptx-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --sysroot=./ \
+// RUN: -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa --offload-arch=gfx908  \
+// RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-HEADERS
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --sysroot=./ \
+// RUN: -fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvidia-cuda --offload-arch=sm_70  \
+// RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-HEADERS
+// RUN:   %clang -### --target=nvptx64-nvidia-cuda -march=sm_70 -nogpulib --sysroot=./ %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-HEADERS
+// RUN:   %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx1030 -nogpulib --sysroot=./ %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-HEADERS
+// CHECK-HEADERS: "-cc1"{{.*}}"-c-isystem" "{{.*}}include/gpu-none-llvm"{{.*}}"-isysroot" "./"
+
+// RUN:   %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx1030 -nogpulib \
+// RUN: -nogpuinc %s 2>&1 | FileCheck %s --check-prefix=CHECK-HEADERS-DISABLED
+// RUN:   %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx1030 

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu created this revision.
zequanwu added reviewers: hans, shafik, rsmith, MaggieYi, gulfem.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

https://github.com/llvm/llvm-project/issues/45481 exposes an issue that an
expression/statement can have valid start location but invalid end location in
some situations.

This confuses `CounterCoverageMappingBuilder` when popping a region from region
stack as if the end location is a macro or include location.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147073

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp


Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -602,6 +602,13 @@
   MostRecentLocation = *StartLoc;
 }
 
+// If the either location is invalid, set it to std::nullopt to avoid
+// letting users of RegionStack think that region has a valid start/end
+// location.
+if (StartLoc && StartLoc->isInvalid())
+  StartLoc = std::nullopt;
+if (EndLoc && EndLoc->isInvalid())
+  EndLoc = std::nullopt;
 RegionStack.emplace_back(Count, FalseCount, StartLoc, EndLoc);
 
 return RegionStack.size() - 1;
@@ -624,7 +631,8 @@
 assert(RegionStack.size() >= ParentIndex && "parent not in stack");
 while (RegionStack.size() > ParentIndex) {
   SourceMappingRegion  = RegionStack.back();
-  if (Region.hasStartLoc()) {
+  if (Region.hasStartLoc() &&
+  (Region.hasEndLoc() || RegionStack[ParentIndex].hasEndLoc())) {
 SourceLocation StartLoc = Region.getBeginLoc();
 SourceLocation EndLoc = Region.hasEndLoc()
 ? Region.getEndLoc()
@@ -691,7 +699,7 @@
 assert(SM.isWrittenInSameFile(Region.getBeginLoc(), EndLoc));
 assert(SpellingRegion(SM, Region).isInSourceOrder());
 SourceRegions.push_back(Region);
-}
+  }
   RegionStack.pop_back();
 }
   }


Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -602,6 +602,13 @@
   MostRecentLocation = *StartLoc;
 }
 
+// If the either location is invalid, set it to std::nullopt to avoid
+// letting users of RegionStack think that region has a valid start/end
+// location.
+if (StartLoc && StartLoc->isInvalid())
+  StartLoc = std::nullopt;
+if (EndLoc && EndLoc->isInvalid())
+  EndLoc = std::nullopt;
 RegionStack.emplace_back(Count, FalseCount, StartLoc, EndLoc);
 
 return RegionStack.size() - 1;
@@ -624,7 +631,8 @@
 assert(RegionStack.size() >= ParentIndex && "parent not in stack");
 while (RegionStack.size() > ParentIndex) {
   SourceMappingRegion  = RegionStack.back();
-  if (Region.hasStartLoc()) {
+  if (Region.hasStartLoc() &&
+  (Region.hasEndLoc() || RegionStack[ParentIndex].hasEndLoc())) {
 SourceLocation StartLoc = Region.getBeginLoc();
 SourceLocation EndLoc = Region.hasEndLoc()
 ? Region.getEndLoc()
@@ -691,7 +699,7 @@
 assert(SM.isWrittenInSameFile(Region.getBeginLoc(), EndLoc));
 assert(SpellingRegion(SM, Region).isInSourceOrder());
 SourceRegions.push_back(Region);
-}
+  }
   RegionStack.pop_back();
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D146973#4228070 , @tra wrote:

> I'm OK with injecting the path *now* with an understanding that it's a 
> short-term "happens to work" way to move forward while we're working on a 
> better solution.

So, the proposed path forward is this. We have `libc` generate its own headers 
so we can have a base implementation. We create these headers with the 
intention of them providing a full interface between the host and device. This 
might mean curating some differences based on whatever the host does, or just 
making sure we choose sizes that are compatible. So these headers are the 
expected interface to the `libc` implementations we support, but we ensure that 
things match between the host and device by only providing interfaces we've 
verified somehow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D147030: [Clang][Driver] Default Generic_GCC::IsIntegratedAssemblerDefault to true

[Brad Smith via Phabricator via cfe-commits]

brad updated this revision to Diff 509084.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147030

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/integrated-as.c


Index: clang/test/Driver/integrated-as.c
===
--- clang/test/Driver/integrated-as.c
+++ clang/test/Driver/integrated-as.c
@@ -12,5 +12,4 @@
 
 // NOFIAS-NOT: cc1as
 // NOFIAS: -cc1
-// NOFIAS: "-fno-verbose-asm"
 // NOFIAS: -no-integrated-as
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2916,44 +2916,12 @@
 
 bool Generic_GCC::IsIntegratedAssemblerDefault() const {
   switch (getTriple().getArch()) {
-  case llvm::Triple::aarch64:
-  case llvm::Triple::aarch64_be:
-  case llvm::Triple::amdgcn:
-  case llvm::Triple::arm:
-  case llvm::Triple::armeb:
-  case llvm::Triple::avr:
-  case llvm::Triple::bpfel:
-  case llvm::Triple::bpfeb:
-  case llvm::Triple::csky:
-  case llvm::Triple::hexagon:
-  case llvm::Triple::lanai:
-  case llvm::Triple::loongarch32:
-  case llvm::Triple::loongarch64:
-  case llvm::Triple::m68k:
-  case llvm::Triple::mips:
-  case llvm::Triple::mipsel:
-  case llvm::Triple::mips64:
-  case llvm::Triple::mips64el:
-  case llvm::Triple::msp430:
-  case llvm::Triple::ppc:
-  case llvm::Triple::ppcle:
-  case llvm::Triple::ppc64:
-  case llvm::Triple::ppc64le:
-  case llvm::Triple::r600:
-  case llvm::Triple::riscv32:
-  case llvm::Triple::riscv64:
-  case llvm::Triple::sparc:
-  case llvm::Triple::sparcel:
-  case llvm::Triple::sparcv9:
-  case llvm::Triple::systemz:
-  case llvm::Triple::thumb:
-  case llvm::Triple::thumbeb:
-  case llvm::Triple::ve:
-  case llvm::Triple::x86:
-  case llvm::Triple::x86_64:
-return true;
-  default:
+  case llvm::Triple::nvptx:
+  case llvm::Triple::nvptx64:
+  case llvm::Triple::xcore:
 return false;
+  default:
+return getTriple().getVendor() != llvm::Triple::Myriad;
   }
 }
 


Index: clang/test/Driver/integrated-as.c
===
--- clang/test/Driver/integrated-as.c
+++ clang/test/Driver/integrated-as.c
@@ -12,5 +12,4 @@
 
 // NOFIAS-NOT: cc1as
 // NOFIAS: -cc1
-// NOFIAS: "-fno-verbose-asm"
 // NOFIAS: -no-integrated-as
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2916,44 +2916,12 @@
 
 bool Generic_GCC::IsIntegratedAssemblerDefault() const {
   switch (getTriple().getArch()) {
-  case llvm::Triple::aarch64:
-  case llvm::Triple::aarch64_be:
-  case llvm::Triple::amdgcn:
-  case llvm::Triple::arm:
-  case llvm::Triple::armeb:
-  case llvm::Triple::avr:
-  case llvm::Triple::bpfel:
-  case llvm::Triple::bpfeb:
-  case llvm::Triple::csky:
-  case llvm::Triple::hexagon:
-  case llvm::Triple::lanai:
-  case llvm::Triple::loongarch32:
-  case llvm::Triple::loongarch64:
-  case llvm::Triple::m68k:
-  case llvm::Triple::mips:
-  case llvm::Triple::mipsel:
-  case llvm::Triple::mips64:
-  case llvm::Triple::mips64el:
-  case llvm::Triple::msp430:
-  case llvm::Triple::ppc:
-  case llvm::Triple::ppcle:
-  case llvm::Triple::ppc64:
-  case llvm::Triple::ppc64le:
-  case llvm::Triple::r600:
-  case llvm::Triple::riscv32:
-  case llvm::Triple::riscv64:
-  case llvm::Triple::sparc:
-  case llvm::Triple::sparcel:
-  case llvm::Triple::sparcv9:
-  case llvm::Triple::systemz:
-  case llvm::Triple::thumb:
-  case llvm::Triple::thumbeb:
-  case llvm::Triple::ve:
-  case llvm::Triple::x86:
-  case llvm::Triple::x86_64:
-return true;
-  default:
+  case llvm::Triple::nvptx:
+  case llvm::Triple::nvptx64:
+  case llvm::Triple::xcore:
 return false;
+  default:
+return getTriple().getVendor() != llvm::Triple::Myriad;
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits