[PATCH] D129105: [clang-format][NFC] Clean up IndentForLevel in LevelIndentTracker

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: curdeius, HazardyKnusperkeks, MyDeveloperDay.
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.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129105

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -59,8 +59,7 @@
 Offset = getIndentOffset(*Line.First);
 // Update the indent level cache size so that we can rely on it
 // having the right size in adjustToUnmodifiedline.
-while (IndentForLevel.size() <= Line.Level)
-  IndentForLevel.push_back(-1);
+skipLine(Line, /*UnknownIndent=*/true);
 if (Line.InPPDirective) {
   unsigned IndentWidth =
   (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
@@ -77,9 +76,11 @@
 
   /// Update the indent state given that \p Line indent should be
   /// skipped.
-  void skipLine(const AnnotatedLine ) {
-while (IndentForLevel.size() <= Line.Level)
-  IndentForLevel.push_back(Indent);
+  void skipLine(const AnnotatedLine , bool UnknownIndent = false) {
+const auto Size = IndentForLevel.size();
+if (Size <= Line.Level)
+  IndentForLevel.insert(IndentForLevel.end(), Line.Level - Size + 1,
+UnknownIndent ? -1 : Indent);
   }
 
   /// Update the level indent to adapt to the given \p Line.
@@ -159,7 +160,7 @@
   const unsigned AdditionalIndent;
 
   /// The indent in characters for each level.
-  std::vector IndentForLevel;
+  SmallVector IndentForLevel;
 
   /// Offset of the current line relative to the indent level.
   ///


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -59,8 +59,7 @@
 Offset = getIndentOffset(*Line.First);
 // Update the indent level cache size so that we can rely on it
 // having the right size in adjustToUnmodifiedline.
-while (IndentForLevel.size() <= Line.Level)
-  IndentForLevel.push_back(-1);
+skipLine(Line, /*UnknownIndent=*/true);
 if (Line.InPPDirective) {
   unsigned IndentWidth =
   (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
@@ -77,9 +76,11 @@
 
   /// Update the indent state given that \p Line indent should be
   /// skipped.
-  void skipLine(const AnnotatedLine ) {
-while (IndentForLevel.size() <= Line.Level)
-  IndentForLevel.push_back(Indent);
+  void skipLine(const AnnotatedLine , bool UnknownIndent = false) {
+const auto Size = IndentForLevel.size();
+if (Size <= Line.Level)
+  IndentForLevel.insert(IndentForLevel.end(), Line.Level - Size + 1,
+UnknownIndent ? -1 : Indent);
   }
 
   /// Update the level indent to adapt to the given \p Line.
@@ -159,7 +160,7 @@
   const unsigned AdditionalIndent;
 
   /// The indent in characters for each level.
-  std::vector IndentForLevel;
+  SmallVector IndentForLevel;
 
   /// Offset of the current line relative to the indent level.
   ///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126266: Mark the file entry invalid, until reread. Invalidate SLocEntry cache, readd it on reread. Do not use translateFile, because it pulls in parts of the pch.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Hmm, I'm concerned with the pieces added purely for testing. Specifically, 
`FileEntriesToReread` and TestHelper friend functions. Need to think how else 
we can test it.

Do you intend to support only the error cases like

  clang-repl> #include "file_with_error.h"
  // error is printed, we edit the file and include it again:
  clang-repl> #include "file_with_error.h"

or do you want to handle re-including files? Something like

  clang-repl> #include "experiments.h"
  // edit the file and include it again:
  clang-repl> #include "experiments.h"

Asking because maybe in the error case we commit to some state too eagerly and 
fixing that sticky eagerness is another option (just guessing, have no idea if 
it is an actual option and if it is "better").


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126266

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

[PATCH] D129068: [AST] Profiling on constraint expression instead of arguments for TypeConstraint in ASTContext::isSameTemplateParameter

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

I don't know enough to say if it is a good approach or not, I'll need to check 
what we can achieve by modifying `Profile` in `ArgLoc.getArgument().Profile`. 
Specifically, I'm interested to see if we can use the same `ASTContext` for 
profile. Also I have a few stylistic comments but I want to figure out 
high-level stuff first.

And I have some ideas about the tests. It might be covered elsewhere but I'm 
curious if there are any not-isSameTemplateParameter test cases? Also it can be 
useless but will it work with the deeper template nesting? Something like 
`C>`.


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

https://reviews.llvm.org/D129068

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

[PATCH] D129104: [Modules] Add ODR Check for concepts

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu created this revision.
ChuanqiXu added reviewers: ilya-biryukov, erichkeane, vsapsai, rsmith.
ChuanqiXu added a project: clang-modules.
Herald added a project: All.
ChuanqiXu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Closing https://github.com/llvm/llvm-project/issues/56310

Previously we don't check the contents of concept so it might merge 
inconsistent results.

Important Note: this patch might break existing code but the behavior might be 
right.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129104

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/Modules/concept_differ.cpp
  clang/test/Modules/concept_differ.cppm

Index: clang/test/Modules/concept_differ.cppm
===
--- /dev/null
+++ clang/test/Modules/concept_differ.cppm
@@ -0,0 +1,43 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -x c++ -std=c++20 %t/A.cppm -I%t -emit-module-interface -o %t/A.pcm
+// RUN: %clang_cc1 -x c++ -std=c++20 %t/B.cppm -I%t -emit-module-interface -o %t/B.pcm
+// RUN: %clang_cc1 -x c++ -std=c++20 -fprebuilt-module-path=%t %t/foo.cpp -verify
+
+//--- foo.h
+template 
+concept A = true;
+
+//--- bar.h
+template 
+concept A = false;
+
+//--- A.cppm
+module;
+#include "foo.h"
+export module A;
+export using ::A;
+
+//--- B.cppm
+module;
+#include "bar.h"
+export module B;
+export using ::A;
+
+//--- foo.cpp
+import A;
+import B;
+
+template  void foo()
+  requires A
+{}   // expected-error 1+{{reference to 'A' is ambiguous}}
+ // expected-note@* 1+{{candidate found by name lookup}}
+template  void bar() {} // expected-error {{reference to 'A' is ambiguous}}
+
+int main() {
+  foo();
+  bar();
+  return 0;
+}
Index: clang/test/Modules/concept_differ.cpp
===
--- /dev/null
+++ clang/test/Modules/concept_differ.cpp
@@ -0,0 +1,37 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -x c++ -std=c++20 -fmodules -fmodules-cache-path=%t -fmodule-map-file=%t/module.map %t/foo.cpp -verify
+
+//--- module.map
+module "foo" {
+export *
+header "foo.h"} module "bar" {
+  export *
+  header "bar.h"
+}
+
+//--- foo.h
+template 
+concept A = true;
+
+//--- bar.h
+template 
+concept A = false;
+
+//--- foo.cpp
+#include "bar.h"
+#include "foo.h"
+
+template  void foo()
+  requires A
+{}   // expected-error 1+{{reference to 'A' is ambiguous}}
+ // expected-note@* 1+{{candidate found by name lookup}}
+template  void bar() {} // expected-error {{reference to 'A' is ambiguous}}
+
+int main() {
+  foo();
+  bar();
+  return 0;
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6524,6 +6524,20 @@
   // and patterns match.
   if (const auto *TemplateX = dyn_cast(X)) {
 const auto *TemplateY = cast(Y);
+
+// ConceptDecl wouldn't be the same if their constraint expression differs.
+if (const auto *ConceptX = dyn_cast(X)) {
+  const auto *ConceptY = dyn_cast(Y);
+  const Expr *XCE = ConceptX->getConstraintExpr();
+  const Expr *YCE = ConceptY->getConstraintExpr();
+  assert(XCE && YCE && "ConceptDecl wihtout constraint expression?");
+  llvm::FoldingSetNodeID XID, YID;
+  XCE->Profile(XID, *this, /*Canonical=*/true);
+  YCE->Profile(YID, *this, /*Canonical=*/true);
+  if (XID != YID)
+return false;
+}
+
 return isSameEntity(TemplateX->getTemplatedDecl(),
 TemplateY->getTemplatedDecl()) &&
isSameTemplateParameterList(TemplateX->getTemplateParameters(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129043: [RISCV][Clang] Teach RISCVEmitter to generate BitCast for pointer operands.

[Yeting Kuo via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG939352b6ec31: [RISCV][Clang] Teach RISCVEmitter to generate 
BitCast for pointer operands. (authored by fakepaper56).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129043

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/include/clang/Support/RISCVVIntrinsicUtils.h
  clang/utils/TableGen/RISCVVEmitter.cpp

Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -105,6 +105,16 @@
 return;
   }
 
+  // Cast pointer operand of vector load intrinsic.
+  for (const auto  : enumerate(RVVI->getInputTypes())) {
+if (I.value()->isPointer()) {
+  assert(RVVI->getIntrinsicTypes().front() == -1 &&
+ "RVVI should be vector load intrinsic.");
+  OS << "  Ops[" << I.index() << "] = Builder.CreateBitCast(Ops[";
+  OS << I.index() << "], ResultType->getPointerTo());\n";
+}
+  }
+
   if (RVVI->isMasked()) {
 if (RVVI->hasVL()) {
   OS << "  std::rotate(Ops.begin(), Ops.begin() + 1, Ops.end() - 1);\n";
Index: clang/include/clang/Support/RISCVVIntrinsicUtils.h
===
--- clang/include/clang/Support/RISCVVIntrinsicUtils.h
+++ clang/include/clang/Support/RISCVVIntrinsicUtils.h
@@ -225,6 +225,8 @@
 return isFloat() && ElementBitwidth == Width;
   }
 
+  bool isPointer() const { return IsPointer; }
+
 private:
   // Verify RVV vector type and set Valid.
   bool verifyType() const;
Index: clang/include/clang/Basic/riscv_vector.td
===
--- clang/include/clang/Basic/riscv_vector.td
+++ clang/include/clang/Basic/riscv_vector.td
@@ -582,18 +582,8 @@
 }
 
 let HasUnMaskedOverloaded = false,
-MaskedPolicy = NonePolicy,
-ManualCodegen = [{
-  IntrinsicTypes = {ResultType, Ops[1]->getType()};
-  Ops[0] = Builder.CreateBitCast(Ops[0], ResultType->getPointerTo());
-}],
-MaskedManualCodegen= [{
-  // Move mask to right before vl.
-  std::rotate(Ops.begin(), Ops.begin() + 1, Ops.end() - 1);
-  IntrinsicTypes = {ResultType, Ops[3]->getType()};
-  Ops[1] = Builder.CreateBitCast(Ops[1], ResultType->getPointerTo());
-}] in {
-  class RVVVLEMaskBuiltin : RVVBuiltin<"m", "mPCUe", "c"> {
+MaskedPolicy = NonePolicy in {
+  class RVVVLEMaskBuiltin : RVVOutBuiltin<"m", "mPCUe", "c"> {
 let Name = "vlm_v";
 let IRName = "vlm";
 let HasMasked = false;
@@ -601,26 +591,15 @@
 }
 
 let HasUnMaskedOverloaded = false,
-ManualCodegen = [{
-  IntrinsicTypes = {ResultType, Ops[1]->getType()};
-  Ops[0] = Builder.CreateBitCast(Ops[0], ResultType->getPointerTo());
-  Ops.insert(Ops.begin(), llvm::UndefValue::get(ResultType));
-}],
-MaskedManualCodegen= [{
-  // Move mask to right before vl.
-  std::rotate(Ops.begin(), Ops.begin() + 1, Ops.end() - 1);
-  Ops.push_back(ConstantInt::get(Ops.back()->getType(), TAIL_UNDISTURBED));
-  IntrinsicTypes = {ResultType, Ops[3]->getType()};
-  Ops[1] = Builder.CreateBitCast(Ops[1], ResultType->getPointerTo());
-}] in {
+UnMaskedPolicy = HasPassthruOperand in {
   multiclass RVVVLEBuiltin types> {
 let Name = NAME # "_v",
 IRName = "vle",
 MaskedIRName ="vle_mask" in {
   foreach type = types in {
-def : RVVBuiltin<"v", "vPCe", type>;
+def : RVVOutBuiltin<"v", "vPCe", type>;
 if !not(IsFloat.val) then {
-  def : RVVBuiltin<"Uv", "UvPCUe", type>;
+  def : RVVOutBuiltin<"Uv", "UvPCUe", type>;
 }
   }
 }
@@ -685,61 +664,39 @@
   IRName = "vlse",
   MaskedIRName ="vlse_mask",
   HasUnMaskedOverloaded = false,
-  ManualCodegen = [{
-IntrinsicTypes = {ResultType, Ops[2]->getType()};
-Ops[0] = Builder.CreateBitCast(Ops[0], ResultType->getPointerTo());
-Ops.insert(Ops.begin(), llvm::UndefValue::get(ResultType));
-  }],
-  MaskedManualCodegen= [{
-// Move mask to right before vl.
-std::rotate(Ops.begin(), Ops.begin() + 1, Ops.end() - 1);
-Ops.push_back(ConstantInt::get(Ops.back()->getType(), TAIL_UNDISTURBED));
-IntrinsicTypes = {ResultType, Ops[4]->getType()};
-Ops[1] = Builder.CreateBitCast(Ops[1], ResultType->getPointerTo());
-  }] in {
+  UnMaskedPolicy = HasPassthruOperand in {
 foreach type = types in {
-  def : RVVBuiltin<"v", "vPCet", type>;
+  def : RVVOutBuiltin<"v", "vPCet", type>;
   if !not(IsFloat.val) then {
-def : RVVBuiltin<"Uv", "UvPCUet", type>;
+def : RVVOutBuiltin<"Uv", "UvPCUet", type>;
   }
 }
   }
 }
 
 multiclass RVVIndexedLoad {
-  let ManualCodegen = [{
-IntrinsicTypes = 

[clang] 939352b - [RISCV][Clang] Teach RISCVEmitter to generate BitCast for pointer operands.

[Yeting Kuo via cfe-commits]

Author: Yeting Kuo
Date: 2022-07-05T11:02:44+08:00
New Revision: 939352b6ec31db4e8defe07856868438fbc5340d

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

LOG: [RISCV][Clang] Teach RISCVEmitter to generate BitCast for pointer operands.

RVV C intrinsics use pointers to scalar for base address and their corresponding
IR intrinsics but use pointers to vector. It makes some vector load intrinsics
need specific ManualCodegen and MaskedManualCodegen to just add bitcast for
transforming to IR.

For simplifying riscv_vector.td, the patch make RISCVEmitter detect pointer
operands and bitcast them.

Reviewed By: kito-cheng

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

Added: 


Modified: 
clang/include/clang/Basic/riscv_vector.td
clang/include/clang/Support/RISCVVIntrinsicUtils.h
clang/utils/TableGen/RISCVVEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/riscv_vector.td 
b/clang/include/clang/Basic/riscv_vector.td
index b11b780ec1f7d..d96020ee40d02 100644
--- a/clang/include/clang/Basic/riscv_vector.td
+++ b/clang/include/clang/Basic/riscv_vector.td
@@ -582,18 +582,8 @@ class IsFloat {
 }
 
 let HasUnMaskedOverloaded = false,
-MaskedPolicy = NonePolicy,
-ManualCodegen = [{
-  IntrinsicTypes = {ResultType, Ops[1]->getType()};
-  Ops[0] = Builder.CreateBitCast(Ops[0], ResultType->getPointerTo());
-}],
-MaskedManualCodegen= [{
-  // Move mask to right before vl.
-  std::rotate(Ops.begin(), Ops.begin() + 1, Ops.end() - 1);
-  IntrinsicTypes = {ResultType, Ops[3]->getType()};
-  Ops[1] = Builder.CreateBitCast(Ops[1], ResultType->getPointerTo());
-}] in {
-  class RVVVLEMaskBuiltin : RVVBuiltin<"m", "mPCUe", "c"> {
+MaskedPolicy = NonePolicy in {
+  class RVVVLEMaskBuiltin : RVVOutBuiltin<"m", "mPCUe", "c"> {
 let Name = "vlm_v";
 let IRName = "vlm";
 let HasMasked = false;
@@ -601,26 +591,15 @@ let HasUnMaskedOverloaded = false,
 }
 
 let HasUnMaskedOverloaded = false,
-ManualCodegen = [{
-  IntrinsicTypes = {ResultType, Ops[1]->getType()};
-  Ops[0] = Builder.CreateBitCast(Ops[0], ResultType->getPointerTo());
-  Ops.insert(Ops.begin(), llvm::UndefValue::get(ResultType));
-}],
-MaskedManualCodegen= [{
-  // Move mask to right before vl.
-  std::rotate(Ops.begin(), Ops.begin() + 1, Ops.end() - 1);
-  Ops.push_back(ConstantInt::get(Ops.back()->getType(), TAIL_UNDISTURBED));
-  IntrinsicTypes = {ResultType, Ops[3]->getType()};
-  Ops[1] = Builder.CreateBitCast(Ops[1], ResultType->getPointerTo());
-}] in {
+UnMaskedPolicy = HasPassthruOperand in {
   multiclass RVVVLEBuiltin types> {
 let Name = NAME # "_v",
 IRName = "vle",
 MaskedIRName ="vle_mask" in {
   foreach type = types in {
-def : RVVBuiltin<"v", "vPCe", type>;
+def : RVVOutBuiltin<"v", "vPCe", type>;
 if !not(IsFloat.val) then {
-  def : RVVBuiltin<"Uv", "UvPCUe", type>;
+  def : RVVOutBuiltin<"Uv", "UvPCUe", type>;
 }
   }
 }
@@ -685,61 +664,39 @@ multiclass RVVVLSEBuiltin types> {
   IRName = "vlse",
   MaskedIRName ="vlse_mask",
   HasUnMaskedOverloaded = false,
-  ManualCodegen = [{
-IntrinsicTypes = {ResultType, Ops[2]->getType()};
-Ops[0] = Builder.CreateBitCast(Ops[0], ResultType->getPointerTo());
-Ops.insert(Ops.begin(), llvm::UndefValue::get(ResultType));
-  }],
-  MaskedManualCodegen= [{
-// Move mask to right before vl.
-std::rotate(Ops.begin(), Ops.begin() + 1, Ops.end() - 1);
-Ops.push_back(ConstantInt::get(Ops.back()->getType(), 
TAIL_UNDISTURBED));
-IntrinsicTypes = {ResultType, Ops[4]->getType()};
-Ops[1] = Builder.CreateBitCast(Ops[1], ResultType->getPointerTo());
-  }] in {
+  UnMaskedPolicy = HasPassthruOperand in {
 foreach type = types in {
-  def : RVVBuiltin<"v", "vPCet", type>;
+  def : RVVOutBuiltin<"v", "vPCet", type>;
   if !not(IsFloat.val) then {
-def : RVVBuiltin<"Uv", "UvPCUet", type>;
+def : RVVOutBuiltin<"Uv", "UvPCUet", type>;
   }
 }
   }
 }
 
 multiclass RVVIndexedLoad {
-  let ManualCodegen = [{
-IntrinsicTypes = {ResultType, Ops[1]->getType(), Ops[2]->getType()};
-Ops[0] = Builder.CreateBitCast(Ops[0], ResultType->getPointerTo());
-Ops.insert(Ops.begin(), llvm::UndefValue::get(ResultType));
-  }],
-  MaskedManualCodegen = [{
-// Move mask to right before vl.
-std::rotate(Ops.begin(), Ops.begin() + 1, Ops.end() - 1);
-Ops.push_back(ConstantInt::get(Ops.back()->getType(), 
TAIL_UNDISTURBED));
-IntrinsicTypes = {ResultType, Ops[2]->getType(), Ops[4]->getType()};
-

[clang] 85318d3 - [NFC] Remove unused test inputs

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-07-05T10:55:24+08:00
New Revision: 85318d3281021f3900ee49338f6a9e2330b4a652

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

LOG: [NFC] Remove unused test inputs

Added: 


Modified: 


Removed: 
clang/test/Modules/Inputs/concept/A.cppm
clang/test/Modules/Inputs/concept/foo.h



diff  --git a/clang/test/Modules/Inputs/concept/A.cppm 
b/clang/test/Modules/Inputs/concept/A.cppm
deleted file mode 100644
index beb370490617b..0
--- a/clang/test/Modules/Inputs/concept/A.cppm
+++ /dev/null
@@ -1,3 +0,0 @@
-module;
-#include "foo.h"
-export module A;

diff  --git a/clang/test/Modules/Inputs/concept/foo.h 
b/clang/test/Modules/Inputs/concept/foo.h
deleted file mode 100644
index cf913a33303ca..0
--- a/clang/test/Modules/Inputs/concept/foo.h
+++ /dev/null
@@ -1,13 +0,0 @@
-#ifndef FOO_H
-#define FOO_H
-
-template 
-concept Range = requires(T ) { t.begin(); };
-
-struct A {
-public:
-  template 
-  using range_type = T;
-};
-
-#endif



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

[PATCH] D129043: [RISCV][Clang] Teach RISCVEmitter to generate BitCast for pointer operands.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng accepted this revision.
kito-cheng added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for clean this up :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129043

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

[PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

BTW, after I applied the patch, the compiler crashes at 
https://github.com/ChuanqiXu9/stdmodules. I would try to add more tests about 
C++20 Modules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[PATCH] D128608: [NFC][ASTImporter] remove the unnecessary condition checks in ASTImporter.cpp

[Shivam Rajput via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG98c6a3c0c220: [NFC][ASTImporter] remove the unnecessary 
condition checks in ASTImporter.cpp (authored by phyBrackets).

Changed prior to commit:
  https://reviews.llvm.org/D128608?vs=440059=442167#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128608

Files:
  clang/include/clang/AST/ASTImportError.h
  clang/lib/AST/ASTImporter.cpp


Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -5667,11 +5667,6 @@
 D2->setPreviousDecl(Recent);
   }
 
-  if (FromTemplated->isCompleteDefinition() &&
-  !ToTemplated->isCompleteDefinition()) {
-// FIXME: Import definition!
-  }
-
   return D2;
 }
 
@@ -5950,11 +5945,6 @@
 ToVarTD->setPreviousDecl(Recent);
   }
 
-  if (DTemplated->isThisDeclarationADefinition() &&
-  !ToTemplated->isThisDeclarationADefinition()) {
-// FIXME: Import definition!
-  }
-
   return ToVarTD;
 }
 
Index: clang/include/clang/AST/ASTImportError.h
===
--- clang/include/clang/AST/ASTImportError.h
+++ clang/include/clang/AST/ASTImportError.h
@@ -19,7 +19,6 @@
 namespace clang {
 
 class ASTImportError : public llvm::ErrorInfo {
-
 public:
   /// \brief Kind of error when importing an AST component.
   enum ErrorKind {


Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -5667,11 +5667,6 @@
 D2->setPreviousDecl(Recent);
   }
 
-  if (FromTemplated->isCompleteDefinition() &&
-  !ToTemplated->isCompleteDefinition()) {
-// FIXME: Import definition!
-  }
-
   return D2;
 }
 
@@ -5950,11 +5945,6 @@
 ToVarTD->setPreviousDecl(Recent);
   }
 
-  if (DTemplated->isThisDeclarationADefinition() &&
-  !ToTemplated->isThisDeclarationADefinition()) {
-// FIXME: Import definition!
-  }
-
   return ToVarTD;
 }
 
Index: clang/include/clang/AST/ASTImportError.h
===
--- clang/include/clang/AST/ASTImportError.h
+++ clang/include/clang/AST/ASTImportError.h
@@ -19,7 +19,6 @@
 namespace clang {
 
 class ASTImportError : public llvm::ErrorInfo {
-
 public:
   /// \brief Kind of error when importing an AST component.
   enum ErrorKind {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 98c6a3c - [NFC][ASTImporter] remove the unnecessary condition checks in ASTImporter.cpp

[via cfe-commits]

Author: phyBrackets
Date: 2022-07-05T08:12:01+05:30
New Revision: 98c6a3c0c2209dd7bdb15fe6c91c507c16990bcf

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

LOG: [NFC][ASTImporter] remove the unnecessary condition checks in 
ASTImporter.cpp

I think that these conditions are unnecessary because in VisitClassTemplateDecl 
we import the definition via the templated CXXRecordDecl and in 
VisitVarTemplateDecl via the templated VarDecl. These are named ToTemplted and 
DTemplated respectively.

Reviewed By: martong

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

Added: 


Modified: 
clang/include/clang/AST/ASTImportError.h
clang/lib/AST/ASTImporter.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTImportError.h 
b/clang/include/clang/AST/ASTImportError.h
index 405790b6ded3f..728314ca0936e 100644
--- a/clang/include/clang/AST/ASTImportError.h
+++ b/clang/include/clang/AST/ASTImportError.h
@@ -19,7 +19,6 @@
 namespace clang {
 
 class ASTImportError : public llvm::ErrorInfo {
-
 public:
   /// \brief Kind of error when importing an AST component.
   enum ErrorKind {

diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index e9730112eaa35..73c3f02e67a85 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -5667,11 +5667,6 @@ ExpectedDecl 
ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
 D2->setPreviousDecl(Recent);
   }
 
-  if (FromTemplated->isCompleteDefinition() &&
-  !ToTemplated->isCompleteDefinition()) {
-// FIXME: Import definition!
-  }
-
   return D2;
 }
 
@@ -5950,11 +5945,6 @@ ExpectedDecl 
ASTNodeImporter::VisitVarTemplateDecl(VarTemplateDecl *D) {
 ToVarTD->setPreviousDecl(Recent);
   }
 
-  if (DTemplated->isThisDeclarationADefinition() &&
-  !ToTemplated->isThisDeclarationADefinition()) {
-// FIXME: Import definition!
-  }
-
   return ToVarTD;
 }
 



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

[PATCH] D129068: [AST] Profiling on constraint expression instead of arguments for TypeConstraint in ASTContext::isSameTemplateParameter

[Chuanqi Xu via Phabricator via cfe-commits]

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

Add comments.


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

https://reviews.llvm.org/D129068

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/Modules/concept.cppm


Index: clang/test/Modules/concept.cppm
===
--- clang/test/Modules/concept.cppm
+++ clang/test/Modules/concept.cppm
@@ -18,6 +18,9 @@
 template 
 concept __member_size = requires(_Tp &) { t.size(); };
 
+template 
+concept C = requires(First x, Second y) { x+y; };
+
 struct A {
 public:
   template 
@@ -29,6 +32,11 @@
   constexpr __integer_like auto operator()(_Tp&& __t) const {
 return __t.size();
   }
+
+  template <__integer_like _Tp, C<_Tp> Sentinel>
+  constexpr _Tp operator()(_Tp &&__t, Sentinel &) const {
+return __t;
+  }
 };
 #endif
 
@@ -51,4 +59,5 @@
 auto operator+(S s) { return 0; }
 };
 __fn{}(S());
+__fn{}(S(), S());
 }
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6245,14 +6245,29 @@
 auto *TYTCArgs = TYTC->getTemplateArgsAsWritten();
 if (TXTCArgs->NumTemplateArgs != TYTCArgs->NumTemplateArgs)
   return false;
-llvm::FoldingSetNodeID XID, YID;
-for (auto  : TXTCArgs->arguments())
-  ArgLoc.getArgument().Profile(XID, X->getASTContext());
-for (auto  : TYTCArgs->arguments())
-  ArgLoc.getArgument().Profile(YID, Y->getASTContext());
-if (XID != YID)
-  return false;
+// We couldn't compare the profiling result for the template
+// args here. Consider the following example in different modules:
+//
+// template <__integer_like _Tp, C<_Tp> Sentinel>
+// constexpr _Tp operator()(_Tp &&__t, Sentinel &) const {
+//   return __t;
+// }
+//
+// When we compare the profiling result for `C<_Tp>` in different
+// modules, it will compare the type of `_Tp` in different modules.
+// However, the type of `_Tp` in different modules refer to different
+// types here naturally. So we couldn't compare the profiling result
+// for the template args directly.
   }
+  llvm::FoldingSetNodeID XID, YID;
+  auto *XConstraint = TXTC->getImmediatelyDeclaredConstraint();
+  auto *YConstraint = TYTC->getImmediatelyDeclaredConstraint();
+  if (XConstraint)
+XConstraint->Profile(XID, *this, /*Canonical=*/true);
+  if (YConstraint)
+YConstraint->Profile(YID, *this, /*Canonical=*/true);
+  if (XID != YID)
+return false;
 }
 return true;
   }


Index: clang/test/Modules/concept.cppm
===
--- clang/test/Modules/concept.cppm
+++ clang/test/Modules/concept.cppm
@@ -18,6 +18,9 @@
 template 
 concept __member_size = requires(_Tp &) { t.size(); };
 
+template 
+concept C = requires(First x, Second y) { x+y; };
+
 struct A {
 public:
   template 
@@ -29,6 +32,11 @@
   constexpr __integer_like auto operator()(_Tp&& __t) const {
 return __t.size();
   }
+
+  template <__integer_like _Tp, C<_Tp> Sentinel>
+  constexpr _Tp operator()(_Tp &&__t, Sentinel &) const {
+return __t;
+  }
 };
 #endif
 
@@ -51,4 +59,5 @@
 auto operator+(S s) { return 0; }
 };
 __fn{}(S());
+__fn{}(S(), S());
 }
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6245,14 +6245,29 @@
 auto *TYTCArgs = TYTC->getTemplateArgsAsWritten();
 if (TXTCArgs->NumTemplateArgs != TYTCArgs->NumTemplateArgs)
   return false;
-llvm::FoldingSetNodeID XID, YID;
-for (auto  : TXTCArgs->arguments())
-  ArgLoc.getArgument().Profile(XID, X->getASTContext());
-for (auto  : TYTCArgs->arguments())
-  ArgLoc.getArgument().Profile(YID, Y->getASTContext());
-if (XID != YID)
-  return false;
+// We couldn't compare the profiling result for the template
+// args here. Consider the following example in different modules:
+//
+// template <__integer_like _Tp, C<_Tp> Sentinel>
+// constexpr _Tp operator()(_Tp &&__t, Sentinel &) const {
+//   return __t;
+// }
+//
+// When we compare the profiling result for `C<_Tp>` in different
+// modules, it will compare the type of `_Tp` in different modules.
+// However, the type of `_Tp` in different modules refer to different
+// types here naturally. So we couldn't compare the profiling result
+// for the template args directly.
   }
+  llvm::FoldingSetNodeID XID, YID;
+  auto *XConstraint = TXTC->getImmediatelyDeclaredConstraint();

[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

[Chuanqi Xu via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129045

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

[PATCH] D128981: [C++20][Modules] Implement include translation.

[Chuanqi Xu via Phabricator via cfe-commits]

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

Then LGTM if all the comments addressed.




Comment at: clang/lib/Lex/PPDirectives.cpp:2226-2227
+
+  // FIXME: We do not have a good way to disambiguate C++ clang modules from
+  // C++ standard modules (other than use/non-use of Header Units).
+  Module *SM = SuggestedModule.getModule();

iains wrote:
> ChuanqiXu wrote:
> > iains wrote:
> > > ChuanqiXu wrote:
> > > > From what @rsmith said in https://reviews.llvm.org/D113391, it looks 
> > > > like the ideal direction is to use C++ clang modules and C++ standard 
> > > > modules together. So it looks like we couldn't disambiguate them from 
> > > > command line options.
> > > Well, I think there is some more debate to have around how to solve this 
> > > problem (i.e. it might be intended that clang++ modules and standard c++ 
> > > modules converge but as things stand we still need to support the cases 
> > > that they have different behaviour, or break existing users) 
> > >  ... - but please let us not have that debate in this patch :-)
> > > 
> > It is not good to break existing users. Generally, a breaking change patch 
> > could be reverted directly... We must care about it to avoid unnecessary 
> > efforts. And it looks like the current implementation wouldn't break any 
> > existing users, right? Since it uses `isHeaderUnit()`. I remember 
> > `HeaderUnit` is introduced by  you so it shouldn't conflict with clang 
> > modules.
> > 
> > BTW, may I ask the behavior is consistency with GCC?
> > It is not good to break existing users. Generally, a breaking change patch 
> > could be reverted directly... We must care about it to avoid unnecessary 
> > efforts. And it looks like the current implementation wouldn't break any 
> > existing users, right? Since it uses `isHeaderUnit()`. I remember 
> > `HeaderUnit` is introduced by  you so it shouldn't conflict with clang 
> > modules.
> 
> correct, in this case, the fact that Header Units are specific to the  C++20 
> implementation (they are quite different from clang header modules) allows us 
> to tell the difference.
> 
> > BTW, may I ask the behavior is consistency with GCC?
> 
> yes, I discussed this with @urnathan (especially that it is very difficult to 
> get consistent behaviour if we were to include-translate in the module 
> purview).
Got it. Thanks.



Comment at: clang/lib/Lex/PPDirectives.cpp:2335
+ IsFirstIncludeOfFile)) {
+// standard modules:
+// If we are not in the GMF, then we textually include only

iains wrote:
> ChuanqiXu wrote:
> > nit: It looks like we prefer to use `C++20 modules` over `standard 
> > modules`, although `standard modules` must be the right term.
> since we are heading for C++23  ... perhaps now would be a good time to start 
> using "standard modules"? (I can change to C++20 modules if there's 
> objection).
> 
It is OK to me. But I feel better if we use `C++ standard modules` since 
modules is a feature lives everywhere.



Comment at: clang/lib/Parse/Parser.cpp:672
+if (!getLangOpts().CPlusPlusModules || !Mod->isHeaderUnit() ||
+getLangOpts().ModulesTS)
+  Actions.ActOnModuleInclude(Loc, Mod);

iains wrote:
> ChuanqiXu wrote:
> > I think we could ignore `getLangOpts().ModulesTS` here.
> well, we agreed that (for the present) we would try to avoid changing the 
> behaviour w.r.t modules-ts (but spend no effort to complete the 
> implementation) - and to remove it from comments; we can then take a pass 
> through to remove the modules-ts behaviour (assuming that no-one is using it!)
> 
Given the previous status of C++ modules, it is hard to believe there is 
existing users for it. So I think it should be fine to remove the support for 
module-ts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128981

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

[PATCH] D129100: [clangd] Support external throttler for preamble builds

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I wanted to send a first version out for feedback even if incomplete.
(I want to do more extensive tests, and likely we should think about having an 
in-tree throttler on by default)

To motivate the design:

- when attempting to acquire a slot, we want to wait on (acquired || shutting 
down).
- This means sharing a condition variable for both events, as we can't wait on 
multiple conditions.
- this leads to the callback-based model for PreambleThrottler, the callback 
lets us encapsulate the use of a shared CV in this way
- the acquire callback naturally accesses the PreambleThread, but it might have 
gone away concurrently. This is awkward to solve in a callback API.
- The chosen solution is to heap-allocate a small object that can own the state 
the callback needs to access


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129100

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

[PATCH] D129100: [clangd] Support external throttler for preamble builds

[Sam McCall via Phabricator via cfe-commits]

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

Building preambles is the most resource-intensive thing clangd does, driving
peak RAM and sustained CPU usage.

In a hosted environment where multiple clangd instances are packed into the same
container, it's useful to be able to limit the *aggregate* resource peaks.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129100

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1372,6 +1372,106 @@
 CheckNoFileActionsSeesLastActiveFile(LastActive);
   }
 }
+
+TEST_F(TUSchedulerTests, PreambleThrottle) {
+  const int NumRequests = 4;
+  // Silly throttler that waits for 4 requests, and services them in reverse.
+  // Doesn't honor cancellation but records it.
+  struct : public PreambleThrottler {
+std::mutex Mu;
+std::vector Acquires;
+std::vector Releases;
+std::vector Cancellations;
+std::vector> PendingRequests;
+
+CancelFn acquire(llvm::StringRef Filename,
+ AcquireCallback Callback) override {
+  // Don't honor cancellations, but record them.
+  auto Cancel = [Filename(std::string(Filename)), this] {
+Cancellations.push_back(Filename);
+  };
+  {
+std::lock_guard Lock(Mu);
+// Record the order we saw acquires.
+Acquires.emplace_back(Filename);
+// If our queue isn't full yet, keep queueing.
+if (PendingRequests.size() + 1 < NumRequests) {
+  PendingRequests.emplace_back(Filename, std::move(Callback));
+  return Cancel;
+}
+  }
+  // OK, we're up to our request limit, allow this last request to proceed.
+  Callback(/*Release=*/[&, Filename(std::string(Filename))] {
+this->release(Filename);
+  });
+  return Cancel;
+}
+
+void reset() {
+  Acquires.clear();
+  Releases.clear();
+  Cancellations.clear();
+  PendingRequests.clear();
+}
+
+  private:
+// When one request finishes, allow the next one to proceed.
+void release(llvm::StringRef Filename) {
+  std::pair Next;
+  {
+std::lock_guard Lock(Mu);
+Releases.emplace_back(Filename);
+if (!PendingRequests.empty()) {
+  Next = std::move(PendingRequests.back());
+  PendingRequests.pop_back();
+}
+  }
+  if (Next.second)
+Next.second([&, Filename(std::string(Next.first))] {
+  this->release(Filename);
+});
+}
+  } Throttler;
+
+  struct CaptureBuiltFilenames : public ParsingCallbacks {
+std::vector 
+CaptureBuiltFilenames(std::vector )
+: Filenames(Filenames) {}
+void onPreambleAST(PathRef Path, llvm::StringRef Version,
+   const CompilerInvocation , ASTContext ,
+   Preprocessor , const CanonicalIncludes &) override {
+  // Deliberately no synchronization.
+  // The PreambleThrottler should serialize these calls, if not then tsan
+  // will find a bug here.
+  Filenames.emplace_back(Path);
+}
+  };
+
+  auto Opts = optsForTest();
+  Opts.AsyncThreadsCount = 2 * NumRequests; // throttler is the bottleneck
+  Opts.PreambleThrottler = 
+
+  {
+std::vector BuiltFilenames;
+TUScheduler S(CDB, Opts,
+  std::make_unique(BuiltFilenames));
+for (unsigned I = 0; I < NumRequests; ++I) {
+  auto Path = testPath(std::to_string(I) + ".cc");
+  S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes);
+}
+ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+
+// Now that we're done, we expect to see preambles were built in reverse.
+EXPECT_THAT(Throttler.PendingRequests, testing::IsEmpty());
+EXPECT_THAT(Throttler.Acquires, testing::SizeIs(NumRequests));
+EXPECT_THAT(BuiltFilenames,
+testing::ElementsAreArray(Throttler.Acquires.rbegin(),
+  Throttler.Acquires.rend()));
+EXPECT_EQ(BuiltFilenames, Throttler.Releases);
+EXPECT_THAT(Throttler.Cancellations, testing::IsEmpty());
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ 

[PATCH] D129064: [clang-format] Avoid crash in LevelIndentTracker.

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:69-70
 } else {
-  IndentForLevel.resize(Line.Level + 1);
+  if (IndentForLevel.size() < Line.Level + 1)
+IndentForLevel.resize(Line.Level + 1);
   Indent = getIndent(Line.Level);

You can delete both lines as the body of the `if` statement is unreachable. 
(After the `while` loop on lines 62-63, `IndentForLevel.size()` is larger than 
`Line.Level`.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129064

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

[clang] b6178cc - [OffloadPackager] Use appropriate kind for LTO bitcode

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2022-07-04T17:34:14-04:00
New Revision: b6178ccfe85238e123ecf29a12af23d3b70bef22

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

LOG: [OffloadPackager] Use appropriate kind for LTO bitcode

Summary:
Currently we just check the extension to set the image kind. This
incorrectly labels the `.o` files created during LTO as object files.
This patch simply adds a check for the bitcode magic bytes instead.

Added: 


Modified: 
clang/tools/clang-offload-packager/CMakeLists.txt
clang/tools/clang-offload-packager/ClangOffloadPackager.cpp

Removed: 




diff  --git a/clang/tools/clang-offload-packager/CMakeLists.txt 
b/clang/tools/clang-offload-packager/CMakeLists.txt
index a781825895a52..accc9486f46ab 100644
--- a/clang/tools/clang-offload-packager/CMakeLists.txt
+++ b/clang/tools/clang-offload-packager/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS 
   ${LLVM_TARGETS_TO_BUILD}
+  BinaryFormat
   Object
   Support)
 

diff  --git a/clang/tools/clang-offload-packager/ClangOffloadPackager.cpp 
b/clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
index 338b63ad0a223..8e98fab2a8a4b 100644
--- a/clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
+++ b/clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
@@ -99,9 +99,14 @@ int main(int argc, const char **argv) {
 llvm::MemoryBuffer::getFileOrSTDIN(KeyAndValue.getValue());
 if (std::error_code EC = ObjectOrErr.getError())
   return reportError(errorCodeToError(EC));
+
+// Clang uses the '.o' suffix for LTO bitcode.
+if (identify_magic((*ObjectOrErr)->getBuffer()) == file_magic::bitcode)
+  ImageBinary.TheImageKind = object::IMG_Bitcode;
+else
+  ImageBinary.TheImageKind = getImageKind(
+  sys::path::extension(KeyAndValue.getValue()).drop_front());
 ImageBinary.Image = std::move(*ObjectOrErr);
-ImageBinary.TheImageKind = getImageKind(
-sys::path::extension(KeyAndValue.getValue()).drop_front());
   } else if (Key == "kind") {
 ImageBinary.TheOffloadKind = getOffloadKind(KeyAndValue.getValue());
   } else {



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

[PATCH] D129097: [clang][dataflow] Handle null pointers of type std::nullptr_t

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua created this revision.
li.zhe.hua added a reviewer: gribozavr2.
Herald added subscribers: martong, tschuett, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
li.zhe.hua requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Treat `std::nullptr_t` as a regular scalar type to avoid tripping
assertions when analyzing code that uses `std::nullptr_t`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129097

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2219,6 +2219,9 @@
   int *FooY = nullptr;
   bool **Bar = nullptr;
   Baz *Baz = nullptr;
+  // Use decltype to avoid needing to include . This generates an
+  // implicit NullToPointer cast of type `std::nullptr_t`.
+  decltype(nullptr) Null = 0;
   // [[p]]
 }
   )";
@@ -2242,6 +2245,9 @@
 const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
 ASSERT_THAT(BazDecl, NotNull());
 
+const ValueDecl *NullDecl = findValueDecl(ASTCtx, "Null");
+ASSERT_THAT(NullDecl, NotNull());
+
 const auto *FooXVal =
 cast(Env.getValue(*FooXDecl, SkipPast::None));
 const auto *FooYVal =
@@ -2250,6 +2256,8 @@
 cast(Env.getValue(*BarDecl, SkipPast::None));
 const auto *BazVal =
 cast(Env.getValue(*BazDecl, SkipPast::None));
+const auto *NullVal =
+cast(Env.getValue(*NullDecl, SkipPast::None));
 
 EXPECT_EQ(FooXVal, FooYVal);
 EXPECT_NE(FooXVal, BarVal);
@@ -2267,6 +2275,11 @@
 const StorageLocation  = BazVal->getPointeeLoc();
 EXPECT_TRUE(isa(BazPointeeLoc));
 EXPECT_THAT(Env.getValue(BazPointeeLoc), IsNull());
+
+const StorageLocation  =
+NullVal->getPointeeLoc();
+EXPECT_TRUE(isa(NullPointeeLoc));
+EXPECT_THAT(Env.getValue(NullPointeeLoc), IsNull());
   });
 }
 
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -24,8 +24,8 @@
 
 StorageLocation &
 DataflowAnalysisContext::getStableStorageLocation(QualType Type) {
-  assert(!Type.isNull());
-  if (Type->isStructureOrClassType() || Type->isUnionType()) {
+  if (!Type.isNull() &&
+  (Type->isStructureOrClassType() || Type->isUnionType())) {
 // FIXME: Explore options to avoid eager initialization of fields as some of
 // them might not be needed for a particular analysis.
 llvm::DenseMap FieldLocs;
@@ -57,8 +57,8 @@
 
 PointerValue &
 DataflowAnalysisContext::getOrCreateNullPointerValue(QualType PointeeType) {
-  assert(!PointeeType.isNull());
-  auto CanonicalPointeeType = PointeeType.getCanonicalType();
+  auto CanonicalPointeeType =
+  PointeeType.isNull() ? PointeeType : PointeeType.getCanonicalType();
   auto Res = NullPointerVals.try_emplace(CanonicalPointeeType, nullptr);
   if (Res.second) {
 auto  = getStableStorageLocation(CanonicalPointeeType);
Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -251,6 +251,17 @@
   bool equivalentBoolValues(BoolValue , BoolValue );
 
 private:
+  struct NullableQualTypeDenseMapInfo : private llvm::DenseMapInfo {
+static QualType getEmptyKey() {
+  // Allow a NULL `QualType` by using a different value as the empty key.
+  return QualType::getFromOpaquePtr(reinterpret_cast(1));
+}
+
+using DenseMapInfo::getHashValue;
+using DenseMapInfo::getTombstoneKey;
+using DenseMapInfo::isEqual;
+  };
+
   /// Adds all constraints of the flow condition identified by `Token` and all
   /// of its transitive dependencies to `Constraints`. `VisitedTokens` is used
   /// to track tokens of flow conditions that were already visited by recursive
@@ -311,7 +322,8 @@
   // required to initialize the `PointeeLoc` field in `PointerValue`. Consider
   // creating a type-independent `NullPointerValue` without a `PointeeLoc`
   // field.
-  llvm::DenseMap NullPointerVals;
+  llvm::DenseMap
+  NullPointerVals;
 
   

[PATCH] D129050: [clang-format] Update documentation

[Nico Weber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1ab37d996ce3: [clang-format] Update documentation (authored 
by thakis).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129050

Files:
  clang/docs/ClangFormat.rst

Index: clang/docs/ClangFormat.rst
===
--- clang/docs/ClangFormat.rst
+++ clang/docs/ClangFormat.rst
@@ -17,7 +17,7 @@
 
 .. code-block:: console
 
-  $ clang-format -help
+  $ clang-format --help
   OVERVIEW: A tool to format C/C++/Java/JavaScript/JSON/Objective-C/Protobuf/C# code.
 
   If no arguments are specified, it formats the code from standard input
@@ -40,9 +40,11 @@
Use with caution, as this might lead to dramatically
differing format depending on an option being
supported or not.
---assume-filename= - Override filename used to determine the language.
- When reading from stdin, clang-format assumes this
- filename to determine the language.
+--assume-filename= - Set filename used to determine the language and to find
+ .clang-format file.
+ Only used when reading from stdin.
+ If this is not passed, the .clang-format file is searched
+ relative to the current working directory when reading stdin.
  Unrecognized filenames are treated as C++.
  supported:
CSharp: .cs
@@ -62,7 +64,7 @@
 --fallback-style=  - The name of the predefined style used as a
  fallback in case clang-format is invoked with
  -style=file, but can not find the .clang-format
- file to use.
+ file to use. Defaults to 'LLVM'.
  Use -fallback-style=none to skip formatting.
 --ferror-limit=  - Set the maximum number of clang-format errors to emit
  before stopping (0 = no limit).
@@ -92,17 +94,19 @@
  determined by the QualifierAlignment style flag
 --sort-includes- If set, overrides the include sorting behavior
  determined by the SortIncludes style flag
---style=   - Coding style, currently supports:
-   LLVM, GNU, Google, Chromium, Microsoft, Mozilla, WebKit.
- Use -style=file to load style configuration from
- .clang-format file located in one of the parent
- directories of the source file (or current
- directory for stdin).
- Use -style=file: to explicitly specify
- the configuration file.
- Use -style="{key: value, ...}" to set specific
- parameters, e.g.:
-   -style="{BasedOnStyle: llvm, IndentWidth: 8}"
+--style=   - Set coding style.  can be:
+ 1. A preset: LLVM, GNU, Google, Chromium, Microsoft,
+Mozilla, WebKit.
+ 2. 'file' to load style configuration from a
+.clang-format file in one of the parent directories
+of the source file (for stdin, see --assume-filename).
+If no .clang-format file is found, falls back to
+--fallback-style.
+--style=file is the default.
+ 3. 'file:' to explicitly specify
+the configuration file.
+ 4. "{key: value, ...}" to set specific parameters, e.g.:
+--style="{BasedOnStyle: llvm, IndentWidth: 8}"
 --verbose  - If set, shows the list of processed files
 
   Generic Options:
@@ -235,37 +239,41 @@
 
 .. code-block:: console
 
-  usage: clang-format-diff.py [-h] [-i] [-p NUM] [-regex PATTERN] [-style STYLE]
+  usage: clang-format-diff.py [-h] [-i] [-p NUM] [-regex PATTERN] [-iregex PATTERN] [-sort-includes] [-v] [-style STYLE]

[clang] 1ab37d9 - [clang-format] Update documentation

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2022-07-04T21:42:08+02:00
New Revision: 1ab37d996ce376129fa435b63ca50246cc8c2f8d

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

LOG: [clang-format] Update documentation

- Update `clang-format --help` output after b1f0efc06acc.
- Update `clang-format-diff.py` help text, which apparently hasn't
  been updated in a while. Since git and svn examples are now part
  of the help text, remove them in the text following the help text.

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

Added: 


Modified: 
clang/docs/ClangFormat.rst

Removed: 




diff  --git a/clang/docs/ClangFormat.rst b/clang/docs/ClangFormat.rst
index 16b316cdf0667..150f0aa009e93 100644
--- a/clang/docs/ClangFormat.rst
+++ b/clang/docs/ClangFormat.rst
@@ -17,7 +17,7 @@ to format C/C++/Java/JavaScript/JSON/Objective-C/Protobuf/C# 
code.
 
 .. code-block:: console
 
-  $ clang-format -help
+  $ clang-format --help
   OVERVIEW: A tool to format 
C/C++/Java/JavaScript/JSON/Objective-C/Protobuf/C# code.
 
   If no arguments are specified, it formats the code from standard input
@@ -40,9 +40,11 @@ to format C/C++/Java/JavaScript/JSON/Objective-C/Protobuf/C# 
code.
Use with caution, as this might lead to 
dramatically

diff ering format depending on an option being
supported or not.
---assume-filename= - Override filename used to determine the 
language.
- When reading from stdin, clang-format 
assumes this
- filename to determine the language.
+--assume-filename= - Set filename used to determine the 
language and to find
+ .clang-format file.
+ Only used when reading from stdin.
+ If this is not passed, the .clang-format 
file is searched
+ relative to the current working directory 
when reading stdin.
  Unrecognized filenames are treated as C++.
  supported:
CSharp: .cs
@@ -62,7 +64,7 @@ to format C/C++/Java/JavaScript/JSON/Objective-C/Protobuf/C# 
code.
 --fallback-style=  - The name of the predefined style used as a
  fallback in case clang-format is invoked 
with
  -style=file, but can not find the 
.clang-format
- file to use.
+ file to use. Defaults to 'LLVM'.
  Use -fallback-style=none to skip 
formatting.
 --ferror-limit=  - Set the maximum number of clang-format 
errors to emit
  before stopping (0 = no limit).
@@ -92,17 +94,19 @@ to format 
C/C++/Java/JavaScript/JSON/Objective-C/Protobuf/C# code.
  determined by the QualifierAlignment 
style flag
 --sort-includes- If set, overrides the include sorting 
behavior
  determined by the SortIncludes style flag
---style=   - Coding style, currently supports:
-   LLVM, GNU, Google, Chromium, Microsoft, 
Mozilla, WebKit.
- Use -style=file to load style 
configuration from
- .clang-format file located in one of the 
parent
- directories of the source file (or current
- directory for stdin).
- Use -style=file: to 
explicitly specify
- the configuration file.
- Use -style="{key: value, ...}" to set 
specific
- parameters, e.g.:
-   -style="{BasedOnStyle: llvm, 
IndentWidth: 8}"
+--style=   - Set coding style.  can be:
+ 1. A preset: LLVM, GNU, Google, Chromium, 
Microsoft,
+Mozilla, WebKit.
+ 2. 'file' to load style configuration 
from a
+.clang-format file in one of the 
parent directories
+of the source file (for stdin, see 
--assume-filename).
+If no .clang-format file is found, 
falls back to
+

[PATCH] D129057: [clang-format] Break on AfterColon only if not followed by comment

[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 rG47bdf53a5dba: [clang-format] Break on AfterColon only if not 
followed by comment (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129057

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7034,6 +7034,10 @@
   "SomeClass::Constructor() :\n"
   "a(aa), aaa() {}",
   Style);
+  verifyFormat(
+  "SomeClass::Constructor() : // NOLINT\n"
+  "a(aa), aaa() {}",
+  Style);
 
   Style.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
   verifyFormat(
@@ -7101,6 +7105,10 @@
"a(aa),\n"
"a(aa) {}",
OnePerLine);
+  verifyFormat("Foo::Foo(int i, int j) : // NOLINT\n"
+   "i(i),// comment\n"
+   "j(j) {}",
+   OnePerLine);
   verifyFormat("MyClass::MyClass(int var) :\n"
"some_var_(var),// 4 space indent\n"
"some_other_var_(var + 1) { // lined up\n"
@@ -7133,6 +7141,17 @@
"// Comment forcing unwanted break.\n"
"() {}",
Style);
+  verifyFormat("Constructor() : // NOLINT\n"
+   "() {}",
+   Style);
+  verifyFormat("Constructor() : // A very long trailing comment that cannot 
fit"
+   " on a single\n"
+   "// line.\n"
+   "() {}",
+   "Constructor() : // A very long trailing comment that cannot 
fit"
+   " on a single line.\n"
+   "() {}",
+   Style);
 
   Style.ColumnLimit = 0;
   verifyFormat("SomeClass::Constructor() :\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4734,7 +4734,7 @@
 // the first list element. Otherwise, it should be placed outside of the
 // list.
 return Left.is(BK_BracedInit) ||
-   (Left.is(TT_CtorInitializerColon) &&
+   (Left.is(TT_CtorInitializerColon) && Right.NewlinesBefore > 0 &&
 Style.BreakConstructorInitializers == 
FormatStyle::BCIS_AfterColon);
   }
   if (Left.is(tok::question) && Right.is(tok::colon))
@@ -4894,8 +4894,10 @@
   if (Right.is(tok::identifier) && Right.Next && 
Right.Next->is(TT_DictLiteral))
 return true;
 
-  if (Left.is(TT_CtorInitializerColon))
-return Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon;
+  if (Left.is(TT_CtorInitializerColon)) {
+return Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon 
&&
+   (!Right.isTrailingComment() || Right.NewlinesBefore > 0);
+  }
   if (Right.is(TT_CtorInitializerColon))
 return Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon;
   if (Left.is(TT_CtorInitializerComma) &&
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -404,6 +404,7 @@
   (State.Column + State.Line->Last->TotalLength - Previous.TotalLength >
getColumnLimit(State) ||
CurrentState.BreakBeforeParameter) &&
+  (!Current.isTrailingComment() || Current.NewlinesBefore > 0) &&
   (Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All ||
Style.BreakConstructorInitializers != FormatStyle::BCIS_BeforeColon ||
Style.ColumnLimit != 0)) {
@@ -793,6 +794,7 @@
   (Previous.is(tok::colon) && Previous.is(TT_ObjCMethodExpr {
 CurrentState.LastSpace = State.Column;
   } else if (Previous.is(TT_CtorInitializerColon) &&
+ (!Current.isTrailingComment() || Current.NewlinesBefore > 0) &&
  Style.BreakConstructorInitializers ==
  FormatStyle::BCIS_AfterColon) {
 CurrentState.Indent = State.Column;
@@ -1032,7 +1034,7 @@
 // be considered bin packing unless the relevant AllowAll option is false 
or
 // this is a dict/object literal.
 bool PreviousIsBreakingCtorInitializerColon =
-Previous.is(TT_CtorInitializerColon) &&
+PreviousNonComment && PreviousNonComment->is(TT_CtorInitializerColon) 
&&
 

[clang] 47bdf53 - [clang-format] Break on AfterColon only if not followed by comment

[via cfe-commits]

Author: owenca
Date: 2022-07-04T12:34:19-07:00
New Revision: 47bdf53a5dbaf16e1080d1cad8f3cc67edaad960

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

LOG: [clang-format] Break on AfterColon only if not followed by comment

Break after a constructor initializer colon only if it's not followed by a
comment on the same line.

Fixes #41128.
Fixes #43246.

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

Added: 


Modified: 
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index 2cb985cdc4e54..e957852ba9859 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -404,6 +404,7 @@ bool ContinuationIndenter::mustBreak(const LineState 
) {
   (State.Column + State.Line->Last->TotalLength - Previous.TotalLength >
getColumnLimit(State) ||
CurrentState.BreakBeforeParameter) &&
+  (!Current.isTrailingComment() || Current.NewlinesBefore > 0) &&
   (Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All ||
Style.BreakConstructorInitializers != FormatStyle::BCIS_BeforeColon ||
Style.ColumnLimit != 0)) {
@@ -793,6 +794,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
, bool DryRun,
   (Previous.is(tok::colon) && Previous.is(TT_ObjCMethodExpr {
 CurrentState.LastSpace = State.Column;
   } else if (Previous.is(TT_CtorInitializerColon) &&
+ (!Current.isTrailingComment() || Current.NewlinesBefore > 0) &&
  Style.BreakConstructorInitializers ==
  FormatStyle::BCIS_AfterColon) {
 CurrentState.Indent = State.Column;
@@ -1032,7 +1034,7 @@ unsigned 
ContinuationIndenter::addTokenOnNewLine(LineState ,
 // be considered bin packing unless the relevant AllowAll option is false 
or
 // this is a dict/object literal.
 bool PreviousIsBreakingCtorInitializerColon =
-Previous.is(TT_CtorInitializerColon) &&
+PreviousNonComment && PreviousNonComment->is(TT_CtorInitializerColon) 
&&
 Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon;
 if (!(Previous.isOneOf(tok::l_paren, tok::l_brace, TT_BinaryOperator) ||
   PreviousIsBreakingCtorInitializerColon) ||

diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 029cb9097871c..98c012994f450 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4734,7 +4734,7 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine 
,
 // the first list element. Otherwise, it should be placed outside of the
 // list.
 return Left.is(BK_BracedInit) ||
-   (Left.is(TT_CtorInitializerColon) &&
+   (Left.is(TT_CtorInitializerColon) && Right.NewlinesBefore > 0 &&
 Style.BreakConstructorInitializers == 
FormatStyle::BCIS_AfterColon);
   }
   if (Left.is(tok::question) && Right.is(tok::colon))
@@ -4894,8 +4894,10 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine 
,
   if (Right.is(tok::identifier) && Right.Next && 
Right.Next->is(TT_DictLiteral))
 return true;
 
-  if (Left.is(TT_CtorInitializerColon))
-return Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon;
+  if (Left.is(TT_CtorInitializerColon)) {
+return Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon 
&&
+   (!Right.isTrailingComment() || Right.NewlinesBefore > 0);
+  }
   if (Right.is(TT_CtorInitializerColon))
 return Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon;
   if (Left.is(TT_CtorInitializerComma) &&

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index c4066253fd1a5..e506d0dbf3a5e 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -7034,6 +7034,10 @@ TEST_F(FormatTest, 
BreakConstructorInitializersAfterColon) {
   "SomeClass::Constructor() :\n"
   "a(aa), aaa() {}",
   Style);
+  verifyFormat(
+  "SomeClass::Constructor() : // NOLINT\n"
+  "a(aa), aaa() {}",
+  Style);
 
   Style.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
   verifyFormat(
@@ -7101,6 +7105,10 @@ TEST_F(FormatTest, 
BreakConstructorInitializersAfterColon) {
"a(aa),\n"
"a(aa) {}",
OnePerLine);
+  verifyFormat("Foo::Foo(int i, int j) : // NOLINT\n"
+   "i(i),

[PATCH] D129057: [clang-format] Break on AfterColon only if not followed by comment

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D129057#3627748 , @curdeius wrote:

> LGTM. Thanks for addressing my comments.

Thank you for your quick review and good comments as always!




Comment at: clang/unittests/Format/FormatTest.cpp:7136
Style);
+  verifyFormat("Constructor() : // NOLINT\n"
+   "() {}",

curdeius wrote:
> How about very long comments? They don't get split now? Please add a test case
Now trailing comments don't wrap after the colon, but long ones still get split 
and aligned as usual.


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

https://reviews.llvm.org/D129057

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

[PATCH] D129093: [pseudo] Eliminate LRTable::Action. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added a subscriber: mgrang.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, alextsao1999.
Herald added a project: clang-tools-extra.

The last remaining uses are in tests/test builders.
Replace with a builder struct.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129093

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
  clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
  clang-tools-extra/pseudo/unittests/GLRTest.cpp
  clang-tools-extra/pseudo/unittests/LRTableTest.cpp

Index: clang-tools-extra/pseudo/unittests/LRTableTest.cpp
===
--- clang-tools-extra/pseudo/unittests/LRTableTest.cpp
+++ clang-tools-extra/pseudo/unittests/LRTableTest.cpp
@@ -20,7 +20,7 @@
 
 using llvm::ValueIs;
 using testing::ElementsAre;
-using Action = LRTable::Action;
+using StateID = LRTable::StateID;
 
 TEST(LRTable, Builder) {
   std::vector GrammarDiags;
@@ -38,22 +38,20 @@
   SymbolID Identifier = tokenSymbol(tok::identifier);
   SymbolID Plus = tokenSymbol(tok::plus);
 
+  LRTable::Builder B(G);
   //   eof  IDENT   term
   // +---++---+--+
   // |state0 || s0|  |
   // |state1 ||   | g3   |
   // |state2 ||   |  |
   // +---++---+--+---
-  std::vector Entries = {
-  {/* State */ 0, Identifier, Action::shift(0)},
-  {/* State */ 1, Term, Action::goTo(3)},
-  };
-  std::vector ReduceEntries = {
-  {/*State=*/0, /*Rule=*/0},
-  {/*State=*/1, /*Rule=*/2},
-  {/*State=*/2, /*Rule=*/1},
-  };
-  LRTable T = LRTable::buildForTests(G, Entries, ReduceEntries);
+  B.Transition[{StateID{0}, Identifier}] = StateID{0};
+  B.Transition[{StateID{1}, Term}] = StateID{3};
+  B.Reduce[StateID{0}].insert(RuleID{0});
+  B.Reduce[StateID{1}].insert(RuleID{2});
+  B.Reduce[StateID{2}].insert(RuleID{1});
+  LRTable T = std::move(B).build();
+
   EXPECT_EQ(T.getShiftState(0, Eof), llvm::None);
   EXPECT_THAT(T.getShiftState(0, Identifier), ValueIs(0));
   EXPECT_THAT(T.getReduceRules(0), ElementsAre(0));
Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -30,7 +30,7 @@
 
 namespace {
 
-using Action = LRTable::Action;
+using StateID = LRTable::StateID;
 using testing::AllOf;
 using testing::ElementsAre;
 using testing::UnorderedElementsAre;
@@ -122,13 +122,11 @@
/*Parents=*/{GSSNode0});
 
   buildGrammar({}, {}); // Create a fake empty grammar.
-  TestLang.Table =
-  LRTable::buildForTests(TestLang.G, /*Entries=*/{
- {1, tokenSymbol(tok::semi), Action::shift(4)},
- {2, tokenSymbol(tok::semi), Action::shift(4)},
- {3, tokenSymbol(tok::semi), Action::shift(5)},
- },
- {});
+  LRTable::Builder B(TestLang.G);
+  B.Transition[{StateID{1}, tokenSymbol(tok::semi)}] = StateID{4};
+  B.Transition[{StateID{2}, tokenSymbol(tok::semi)}] = StateID{4};
+  B.Transition[{StateID{3}, tokenSymbol(tok::semi)}] = StateID{5};
+  TestLang.Table = std::move(B).build();
 
   ForestNode  = Arena.createTerminal(tok::semi, 0);
   std::vector NewHeads;
@@ -152,17 +150,12 @@
   //└--3(enum-name) // 3 is goto(0, enum-name)
   buildGrammar({"class-name", "enum-name"},
{"class-name := IDENTIFIER", "enum-name := IDENTIFIER"});
-
-  TestLang.Table = LRTable::buildForTests(
-  TestLang.G,
-  {
-  {/*State=*/0, id("class-name"), Action::goTo(2)},
-  {/*State=*/0, id("enum-name"), Action::goTo(3)},
-  },
-  {
-  {/*State=*/1, ruleFor("class-name")},
-  {/*State=*/1, ruleFor("enum-name")},
-  });
+  LRTable::Builder B(TestLang.G);
+  B.Transition[{StateID{0}, id("class-name")}] = StateID{2};
+  B.Transition[{StateID{0}, id("enum-name")}] = StateID{3};
+  B.Reduce[StateID{1}].insert(ruleFor("class-name"));
+  B.Reduce[StateID{1}].insert(ruleFor("enum-name"));
+  TestLang.Table = std::move(B).build();
 
   const auto *GSSNode0 =
   GSStack.addNode(/*State=*/0, /*ForestNode=*/nullptr, /*Parents=*/{});
@@ -202,15 +195,12 @@
   /*State=*/4, (tok::star, /*TokenIndex=*/1),
   /*Parents=*/{GSSNode2, GSSNode3});
 
-  TestLang.Table = LRTable::buildForTests(
-  TestLang.G,
-  {
-  {/*State=*/2, id("ptr-operator"), Action::goTo(/*NextState=*/5)},
-  {/*State=*/3, id("ptr-operator"), Action::goTo(/*NextState=*/6)},
-  },
-  {
-  {/*State=*/4, ruleFor("ptr-operator")},
-  });
+  LRTable::Builder B(TestLang.G);
+  

[PATCH] D128485: [pseudo] Store shift and goto actions in a compact structure with faster lookup.

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sammccall marked 3 inline comments as done.
Closed by commit rGb37dafd5dc83: [pseudo] Store shift and goto actions in a 
compact structure with faster lookup. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D128485?vs=439589=442123#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128485

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
  clang-tools-extra/pseudo/unittests/LRTableTest.cpp

Index: clang-tools-extra/pseudo/unittests/LRTableTest.cpp
===
--- clang-tools-extra/pseudo/unittests/LRTableTest.cpp
+++ clang-tools-extra/pseudo/unittests/LRTableTest.cpp
@@ -60,7 +60,7 @@
 
   EXPECT_EQ(T.getShiftState(1, Eof), llvm::None);
   EXPECT_EQ(T.getShiftState(1, Identifier), llvm::None);
-  EXPECT_EQ(T.getGoToState(1, Term), 3);
+  EXPECT_THAT(T.getGoToState(1, Term), ValueIs(3));
   EXPECT_THAT(T.getReduceRules(1), ElementsAre(2));
 
   // Verify the behaivor for other non-available-actions terminals.
Index: clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
===
--- clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
+++ clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
@@ -45,49 +45,24 @@
   llvm::DenseMap> Reduces;
   std::vector> FollowSets;
 
-  LRTable build(unsigned NumStates) && {
-// E.g. given the following parsing table with 3 states and 3 terminals:
-//
-//ab c
-// +---++---+-+
-// |state0 || s0,r0 | |
-// |state1 | acc|   | |
-// |state2 ||  r1   | |
-// +---++---+-+
-//
-// The final LRTable:
-//  - StateOffset: [s0] = 0, [s1] = 2, [s2] = 3, [sentinel] = 4
-//  - Symbols: [ b,   b,   a,  b]
-//Actions: [ s0, r0, acc, r1]
-//   ~~ range for state 0
-//    range for state 1
-//~~ range for state 2
-// First step, we sort all entries by (State, Symbol, Action).
-std::vector Sorted(Entries.begin(), Entries.end());
-llvm::sort(Sorted, [](const Entry , const Entry ) {
-  return std::forward_as_tuple(L.State, L.Symbol, L.Act.opaque()) <
- std::forward_as_tuple(R.State, R.Symbol, R.Act.opaque());
-});
-
+  LRTable build(unsigned NumStates, unsigned NumNonterminals) && {
 LRTable Table;
-Table.Actions.reserve(Sorted.size());
-Table.Symbols.reserve(Sorted.size());
-// We are good to finalize the States and Actions.
-for (const auto  : Sorted) {
-  Table.Actions.push_back(E.Act);
-  Table.Symbols.push_back(E.Symbol);
-}
-// Initialize the terminal and nonterminal offset, all ranges are empty by
-// default.
-Table.StateOffset = std::vector(NumStates + 1, 0);
-size_t SortedIndex = 0;
-for (StateID State = 0; State < Table.StateOffset.size(); ++State) {
-  Table.StateOffset[State] = SortedIndex;
-  while (SortedIndex < Sorted.size() && Sorted[SortedIndex].State == State)
-++SortedIndex;
-}
 Table.StartStates = std::move(StartStates);
 
+// Compile the goto and shift actions into transition tables.
+llvm::DenseMap Gotos;
+llvm::DenseMap Shifts;
+for (const auto  : Entries) {
+  if (E.Act.kind() == Action::Shift)
+Shifts.try_emplace(shiftIndex(E.State, E.Symbol, NumStates),
+   E.Act.getShiftState());
+  else if (E.Act.kind() == Action::GoTo)
+Gotos.try_emplace(gotoIndex(E.State, E.Symbol, NumStates),
+  E.Act.getGoToState());
+}
+Table.Shifts = TransitionTable(Shifts, NumStates * NumTerminals);
+Table.Gotos = TransitionTable(Gotos, NumStates * NumNonterminals);
+
 // Compile the follow sets into a bitmap.
 Table.FollowSets.resize(tok::NUM_TOKENS * FollowSets.size());
 for (SymbolID NT = 0; NT < FollowSets.size(); ++NT)
@@ -128,7 +103,8 @@
   for (const ReduceEntry  : Reduces)
 Build.Reduces[E.State].insert(E.Rule);
   Build.FollowSets = followSets(G);
-  return std::move(Build).build(/*NumStates=*/MaxState + 1);
+  return std::move(Build).build(/*NumStates=*/MaxState + 1,
+G.table().Nonterminals.size());
 }
 
 LRTable LRTable::buildSLR(const Grammar ) {
@@ -156,7 +132,8 @@
 Build.Reduces[SID].insert(I.rule());
 }
   }
-  return std::move(Build).build(Graph.states().size());
+  return std::move(Build).build(Graph.states().size(),
+G.table().Nonterminals.size());
 

[clang-tools-extra] b37dafd - [pseudo] Store shift and goto actions in a compact structure with faster lookup.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-07-04T19:40:04+02:00
New Revision: b37dafd5dc83a5f1fc4ca7e37e4944364ff9d5b7

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

LOG: [pseudo] Store shift and goto actions in a compact structure with faster 
lookup.

The actions table is very compact but the binary search to find the
correct action is relatively expensive.
A hashtable is faster but pretty large (64 bits per value, plus empty
slots, and lookup is constant time but not trivial due to collisions).

The structure in this patch uses 1.25 bits per entry (whether present or absent)
plus the size of the values, and lookup is trivial.

The Shift table is 119KB = 27KB values + 92KB keys.
The Goto table is 86KB = 30KB values + 57KB keys.
(Goto has a smaller keyspace as #nonterminals < #terminals, and more entries).

This patch improves glrParse speed by 28%: 4.69 => 5.99 MB/s
Overall the table grows by 60%: 142 => 228KB.

By comparison, DenseMap is "only" 16% faster (5.43 MB/s),
and results in a 285% larger table (547 KB) vs the baseline.

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

Added: 


Modified: 
clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
clang-tools-extra/pseudo/lib/GLR.cpp
clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
clang-tools-extra/pseudo/unittests/LRTableTest.cpp

Removed: 




diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
index d480956704960..cd183b552d6f9 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
@@ -40,6 +40,8 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/Support/Capacity.h"
+#include "llvm/Support/MathExtras.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 #include 
 
@@ -123,12 +125,18 @@ class LRTable {
 
   // Returns the state after we reduce a nonterminal.
   // Expected to be called by LR parsers.
-  // REQUIRES: Nonterminal is valid here.
-  StateID getGoToState(StateID State, SymbolID Nonterminal) const;
+  // If the nonterminal is invalid here, returns None.
+  llvm::Optional getGoToState(StateID State,
+   SymbolID Nonterminal) const {
+return Gotos.get(gotoIndex(State, Nonterminal, numStates()));
+  }
   // Returns the state after we shift a terminal.
   // Expected to be called by LR parsers.
   // If the terminal is invalid here, returns None.
-  llvm::Optional getShiftState(StateID State, SymbolID Terminal) 
const;
+  llvm::Optional getShiftState(StateID State,
+SymbolID Terminal) const {
+return Shifts.get(shiftIndex(State, Terminal, numStates()));
+  }
 
   // Returns the possible reductions from a state.
   //
@@ -164,9 +172,7 @@ class LRTable {
   StateID getStartState(SymbolID StartSymbol) const;
 
   size_t bytes() const {
-return sizeof(*this) + llvm::capacity_in_bytes(Actions) +
-   llvm::capacity_in_bytes(Symbols) +
-   llvm::capacity_in_bytes(StateOffset) +
+return sizeof(*this) + Gotos.bytes() + Shifts.bytes() +
llvm::capacity_in_bytes(Reduces) +
llvm::capacity_in_bytes(ReduceOffset) +
llvm::capacity_in_bytes(FollowSets);
@@ -194,22 +200,92 @@ class LRTable {
llvm::ArrayRef);
 
 private:
-  // Looks up actions stored in the generic table.
-  llvm::ArrayRef find(StateID State, SymbolID Symbol) const;
-
-  // Conceptually the LR table is a multimap from (State, SymbolID) => Action.
-  // Our physical representation is quite 
diff erent for compactness.
-
-  // Index is StateID, value is the offset into Symbols/Actions
-  // where the entries for this state begin.
-  // Give a state id, the corresponding half-open range of Symbols/Actions is
-  // [StateOffset[id], StateOffset[id+1]).
-  std::vector StateOffset;
-  // Parallel to Actions, the value is SymbolID (columns of the matrix).
-  // Grouped by the StateID, and only subranges are sorted.
-  std::vector Symbols;
-  // A flat list of available actions, sorted by (State, SymbolID).
-  std::vector Actions;
+  unsigned numStates() const { return ReduceOffset.size() - 1; }
+
+  // A map from unsigned key => StateID, used to store actions.
+  // The keys should be sequential but the values are somewhat sparse.
+  //
+  // In practice, the keys encode (origin state, symbol) pairs, and the values
+  // are the state we should move to after seeing that symbol.
+  //
+  // We store one bit for presence/absence of the value for each key.
+  // At every 64th key, we store the offset into the 

[PATCH] D103313: [RISCV][Clang] Add support for Zmmul extension

[ksyx via Phabricator via cfe-commits]

ksyx added a comment.

ping :)


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

https://reviews.llvm.org/D103313

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

[PATCH] D111081: [clang] [MinGW] Fix paths on Gentoo

[Mark Harmstone via Phabricator via cfe-commits]

maharmstone added a comment.

In D111081#3627852 , @MaskRay wrote:

> Does Gentoo patch upstream to do something different? (`.../g++-v10/...`) 
> Well, I wish that Gentoo does not do this.
> This adds complexity to Clang which in practice tries to provide some drop-in 
> replacement ability.

Clang doesn't work at all on Gentoo at the moment with any of the mingw 
targets, hence this patch.

As I said in the top comment, these changes are mirroring what's in 
clang/lib/Driver/ToolChains/Gnu.cpp for the Linux triplets.


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

https://reviews.llvm.org/D111081

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

[clang] 9eb6572 - [LV] Add back CantReorderMemOps remark.

[Florian Hahn via cfe-commits]

Author: Florian Hahn
Date: 2022-07-04T17:23:47+01:00
New Revision: 9eb657278611665433d30fb37979d1df48af2ac8

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

LOG: [LV] Add back CantReorderMemOps remark.

Add back remark unintentionally dropped by 644a965c1efef68f.

I will add a LV test separately, so we do not have to rely on a Clang
test to catch this.

Added: 


Modified: 
clang/test/Frontend/optimization-remark-options.c
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

Removed: 




diff  --git a/clang/test/Frontend/optimization-remark-options.c 
b/clang/test/Frontend/optimization-remark-options.c
index 3509a388d0f6..96e480d140be 100644
--- a/clang/test/Frontend/optimization-remark-options.c
+++ b/clang/test/Frontend/optimization-remark-options.c
@@ -1,5 +1,5 @@
 // REQUIRES: x86-registered-target
-// RUN: %clang -O1 -fvectorize -target x86_64-unknown-unknown 
-Rpass-analysis=loop-vectorize -emit-llvm -S %s -o - 2>&1 | FileCheck %s
+// RUN: %clang -O1 -fvectorize -target x86_64-unknown-unknown -mllvm 
-vectorize-memory-check-threshold=8 -Rpass-analysis=loop-vectorize -emit-llvm 
-S %s -o - 2>&1 | FileCheck %s
 
 // CHECK: {{.*}}:10:11: remark: loop not vectorized: cannot prove it is safe 
to reorder floating-point operations; allow reordering by specifying '#pragma 
clang loop vectorize(enable)' before the loop or by providing the compiler 
option '-ffast-math'.
 

diff  --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp 
b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index c9e9136bbd3c..b48c3e18def5 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -10514,8 +10514,18 @@ bool LoopVectorizePass::processLoop(Loop *L) {
 bool ForceVectorization =
 Hints.getForce() == LoopVectorizeHints::FK_Enabled;
 if (!ForceVectorization &&
-!areRuntimeChecksProfitable(Checks, VF, L, *PSE.getSE()))
+!areRuntimeChecksProfitable(Checks, VF, L, *PSE.getSE())) {
+  ORE->emit([&]() {
+return OptimizationRemarkAnalysisAliasing(
+   DEBUG_TYPE, "CantReorderMemOps", L->getStartLoc(),
+   L->getHeader())
+   << "loop not vectorized: cannot prove it is safe to reorder "
+  "memory operations";
+  });
+  LLVM_DEBUG(dbgs() << "LV: Too many memory checks needed.\n");
+  Hints.emitRemarkWithHints();
   return false;
+}
   }
 
   // Identify the diagnostic messages that should be produced.



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

[PATCH] D128783: [test] Check for more -fsanitize=array-bounds regressions

[serge via Phabricator via cfe-commits]

serge-sans-paille accepted this revision.
serge-sans-paille added a comment.

I'm fine with these tests has they reflect current implementation. But beware 
that ubsan and `-Warray-bounds` don't behave the same wrt. FAM, which I find 
disturbing. I'll discuss that in another review.


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

https://reviews.llvm.org/D128783

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

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 442109.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/CodeGen/arm-target-features.c
  clang/test/Driver/arm-cortex-cpus-2.c
  clang/test/Driver/arm-nofp-disabled-features.c
  clang/test/Driver/arm-nopacbti-disabled-features.c
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Target/ARM/ARM.td
  llvm/test/CodeGen/ARM/build-attributes.ll
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -395,13 +395,19 @@
  ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
  ARM::AEK_FP16,
  "8.1-M.Mainline"),
+ARMCPUTestParams("cortex-m85", "armv8.1-m.main",
+ "fp-armv8-fullfp16-d16",
+ ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_SIMD |
+ ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
+ ARM::AEK_FP16 | ARM::AEK_PACBTI,
+ "8.1-M.Mainline"),
 ARMCPUTestParams("iwmmxt", "iwmmxt", "none", ARM::AEK_NONE, "iwmmxt"),
 ARMCPUTestParams("xscale", "xscale", "none", ARM::AEK_NONE, "xscale"),
 ARMCPUTestParams("swift", "armv7s", "neon-vfpv4",
  ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP,
  "7-S")));
 
-static constexpr unsigned NumARMCPUArchs = 88;
+static constexpr unsigned NumARMCPUArchs = 89;
 
 TEST(TargetParserTest, testARMCPUArchList) {
   SmallVector List;
Index: llvm/test/CodeGen/ARM/build-attributes.ll
===
--- llvm/test/CodeGen/ARM/build-attributes.ll
+++ llvm/test/CodeGen/ARM/build-attributes.ll
@@ -235,6 +235,8 @@
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+mve.fp | FileCheck %s --check-prefix=ARMv81M-MAIN-MVEFP
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+pacbti | FileCheck %s --check-prefix=ARMv81M-MAIN-PACBTI
 ; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m55 | FileCheck %s --check-prefix=CORTEX-M55
+; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m85 | FileCheck %s --check-prefix=CORTEX-M85
+; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m85+nopacbti | FileCheck %s --check-prefix=CHECK-NO-PACBTI
 
 ; CPU-SUPPORTED-NOT: is not a recognized processor for this target
 
@@ -1748,6 +1750,23 @@
 ; CORTEX-M55: .eabi_attribute 38, 1
 ; CORTEX-M55: .eabi_attribute 14, 0
 
+; CORTEX-M85: .cpu cortex-m85
+; CORTEX-M85: .eabi_attribute 6, 21   @ Tag_CPU_arch
+; CORTEX-M85: .eabi_attribute 7, 77   @ Tag_CPU_arch_profile
+; CORTEX-M85: .eabi_attribute 8, 0@ Tag_ARM_ISA_use
+; CORTEX-M85: .eabi_attribute 9, 3@ Tag_THUMB_ISA_use
+; CORTEX-M85: .fpufpv5-d16
+; CORTEX-M85: .eabi_attribute 36, 1   @ Tag_FP_HP_extension
+; CORTEX-M85: .eabi_attribute 48, 2   @ Tag_MVE_arch
+; CORTEX-M85: .eabi_attribute 46, 1   @ Tag_DSP_extension
+; CORTEX-M85: .eabi_attribute 34, 1   @ Tag_CPU_unaligned_access
+; CORTEX-M85: .eabi_attribute 50, 2   @ Tag_PAC_extension
+; CORTEX-M85: .eabi_attribute 52, 2   @ Tag_BTI_extension
+
+; CHECK-NO-PACBTI-NOT: .eabi_attribute 50
+; CHECK-NO-PACBTI-NOT: .eabi_attribute 52
+
+
 define i32 @f(i64 %z) {
 ret i32 0
 }
Index: llvm/lib/Target/ARM/ARM.td
===
--- llvm/lib/Target/ARM/ARM.td
+++ llvm/lib/Target/ARM/ARM.td
@@ -1450,6 +1450,13 @@
  HasMVEFloatOps,
  FeatureFixCMSE_CVE_2021_35465]>;
 
+def : ProcessorModel<"cortex-m85", CortexM7Model,   [ARMv81mMainline,
+ FeatureDSP,
+ FeatureFPARMv8_D16,
+ FeaturePACBTI,
+ FeatureUseMISched,
+ HasMVEFloatOps]>;
+
 def : ProcNoItin<"cortex-a32",   [ARMv8a,
  FeatureHWDivThumb,
  FeatureHWDivARM,
Index: llvm/include/llvm/Support/ARMTargetParser.def
===
--- llvm/include/llvm/Support/ARMTargetParser.def
+++ llvm/include/llvm/Support/ARMTargetParser.def
@@ -303,6 +303,9 @@
 ARM_CPU_NAME("cortex-m35p", ARMV8MMainline, 

[PATCH] D129070: [clang-tidy] Fixed an issue in readability-identifier-naming not using options specified.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 442101.
njames93 added a comment.

Small tweak.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129070

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-empty-options.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-empty-options.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-empty-options.cpp
@@ -0,0 +1,12 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: { \
+// RUN: readability-identifier-naming.GlobalConstantPrefix: "", \
+// RUN: readability-identifier-naming.GlobalVariablePrefix: g_ }}'
+
+int BadGlobalVariable;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global 
variable 'BadGlobalVariable' [readability-identifier-naming]
+// CHECK-FIXES: int g_BadGlobalVariable;
+int g_GoodGlobalVariable;
+
+const int GoodGlobalConstant = 0;
+const int g_IgnoreGlobalConstant = 0;
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -234,6 +234,11 @@
 - Fixed incorrect suggestions for :doc:`readability-container-size-empty
   ` when smart pointers 
are involved.
 
+- Fixed an issue in :doc:`readability-identifier-naming
+  ` when specifying an empty
+  string for ``Prefix`` or ``Suffix`` options could result in the style not
+  being used.
+
 - Fixed a false positive in :doc:`readability-non-const-parameter
   ` when the parameter is
   referenced by an lvalue.
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -69,8 +69,8 @@
   struct NamingStyle {
 NamingStyle() = default;
 
-NamingStyle(llvm::Optional Case, const std::string ,
-const std::string , const std::string ,
+NamingStyle(llvm::Optional Case, StringRef Prefix,
+StringRef Suffix, StringRef IgnoredRegexpStr,
 HungarianPrefixType HPType);
 NamingStyle(const NamingStyle ) = delete;
 NamingStyle =(NamingStyle &) = default;
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -228,9 +228,8 @@
 // clang-format on
 
 IdentifierNamingCheck::NamingStyle::NamingStyle(
-llvm::Optional Case,
-const std::string , const std::string ,
-const std::string , HungarianPrefixType HPType)
+llvm::Optional Case, StringRef Prefix,
+StringRef Suffix, StringRef IgnoredRegexpStr, HungarianPrefixType HPType)
 : Case(Case), Prefix(Prefix), Suffix(Suffix),
   IgnoredRegexpStr(IgnoredRegexpStr), HPType(HPType) {
   if (!IgnoredRegexpStr.empty()) {
@@ -263,22 +262,21 @@
 
 memcpy([StyleSize], "IgnoredRegexp", 13);
 StyleString.truncate(StyleSize + 13);
-StringRef IgnoredRegexpStr = Options.get(StyleString, "");
+Optional IgnoredRegexpStr = Options.get(StyleString);
 memcpy([StyleSize], "Prefix", 6);
 StyleString.truncate(StyleSize + 6);
-std::string Prefix(Options.get(StyleString, ""));
+Optional Prefix(Options.get(StyleString));
 // Fast replacement of [Pre]fix -> [Suf]fix.
 memcpy([StyleSize], "Suf", 3);
-std::string Postfix(Options.get(StyleString, ""));
+Optional Postfix(Options.get(StyleString));
 memcpy([StyleSize], "Case", 4);
 StyleString.pop_back_n(2);
-auto CaseOptional =
+Optional CaseOptional =
 Options.get(StyleString);
 
-if (CaseOptional || !Prefix.empty() || !Postfix.empty() ||
-!IgnoredRegexpStr.empty() || HPTOpt)
-  Styles[I].emplace(std::move(CaseOptional), std::move(Prefix),
-std::move(Postfix), IgnoredRegexpStr.str(),
+if (CaseOptional || Prefix || Postfix || IgnoredRegexpStr || HPTOpt)
+  Styles[I].emplace(std::move(CaseOptional), Prefix.getValueOr(""),
+Postfix.getValueOr(""), 
IgnoredRegexpStr.getValueOr(""),
 HPTOpt.value_or(IdentifierNamingCheck::HPT_Off));
   }
   bool IgnoreMainLike = Options.get("IgnoreMainLikeFunctions", false);


Index: 

[PATCH] D127874: [analyzer] Reimplement UnreachableCodeChecker using worklists

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D127874#3628112 , @martong wrote:

>> I don't think any of the issues mentioned in this patch relates to strongly 
>> connected components, thus I don't think I can answer to this question.
>
> In your example above, repeated here:
>
>   #6(entry)  #2(goto a;)
>|  |^
>   #5(goto end;)   | \
>| #4(goto b;) |
>   #1(end:)|  |
>| #3(goto c;) |
>   #0(exit) \/
>
> [#2, #4, #3] is a strongly connected (and unreachable) component  of the CFG, 
> isn't it?

Upsz, I've made a mistake. I wanted to write **connected components** without 
the **strongly** adverb. Please remove the **strongly** from all my comments 
above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127874

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

[PATCH] D129074: [pseudo] Use the prebuilt cxx grammar for the lit tests, NFC.

[Sam McCall via Phabricator via cfe-commits]

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

Up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129074

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

[PATCH] D128981: [C++20][Modules] Implement include translation.

[Iain Sandoe via Phabricator via cfe-commits]

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



Comment at: clang/lib/Lex/PPDirectives.cpp:2226-2227
+
+  // FIXME: We do not have a good way to disambiguate C++ clang modules from
+  // C++ standard modules (other than use/non-use of Header Units).
+  Module *SM = SuggestedModule.getModule();

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > From what @rsmith said in https://reviews.llvm.org/D113391, it looks like 
> > > the ideal direction is to use C++ clang modules and C++ standard modules 
> > > together. So it looks like we couldn't disambiguate them from command 
> > > line options.
> > Well, I think there is some more debate to have around how to solve this 
> > problem (i.e. it might be intended that clang++ modules and standard c++ 
> > modules converge but as things stand we still need to support the cases 
> > that they have different behaviour, or break existing users) 
> >  ... - but please let us not have that debate in this patch :-)
> > 
> It is not good to break existing users. Generally, a breaking change patch 
> could be reverted directly... We must care about it to avoid unnecessary 
> efforts. And it looks like the current implementation wouldn't break any 
> existing users, right? Since it uses `isHeaderUnit()`. I remember 
> `HeaderUnit` is introduced by  you so it shouldn't conflict with clang 
> modules.
> 
> BTW, may I ask the behavior is consistency with GCC?
> It is not good to break existing users. Generally, a breaking change patch 
> could be reverted directly... We must care about it to avoid unnecessary 
> efforts. And it looks like the current implementation wouldn't break any 
> existing users, right? Since it uses `isHeaderUnit()`. I remember 
> `HeaderUnit` is introduced by  you so it shouldn't conflict with clang 
> modules.

correct, in this case, the fact that Header Units are specific to the  C++20 
implementation (they are quite different from clang header modules) allows us 
to tell the difference.

> BTW, may I ask the behavior is consistency with GCC?

yes, I discussed this with @urnathan (especially that it is very difficult to 
get consistent behaviour if we were to include-translate in the module purview).



Comment at: clang/lib/Lex/PPDirectives.cpp:2335
+ IsFirstIncludeOfFile)) {
+// standard modules:
+// If we are not in the GMF, then we textually include only

ChuanqiXu wrote:
> nit: It looks like we prefer to use `C++20 modules` over `standard modules`, 
> although `standard modules` must be the right term.
since we are heading for C++23  ... perhaps now would be a good time to start 
using "standard modules"? (I can change to C++20 modules if there's objection).




Comment at: clang/lib/Parse/Parser.cpp:672
+if (!getLangOpts().CPlusPlusModules || !Mod->isHeaderUnit() ||
+getLangOpts().ModulesTS)
+  Actions.ActOnModuleInclude(Loc, Mod);

ChuanqiXu wrote:
> I think we could ignore `getLangOpts().ModulesTS` here.
well, we agreed that (for the present) we would try to avoid changing the 
behaviour w.r.t modules-ts (but spend no effort to complete the implementation) 
- and to remove it from comments; we can then take a pass through to remove the 
modules-ts behaviour (assuming that no-one is using it!)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128981

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

[PATCH] D128981: [C++20][Modules] Implement include translation.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 442085.
iains marked 7 inline comments as done.
iains added a comment.

rebased, addressed review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128981

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Parse/Parser.cpp
  clang/test/Modules/cxx20-include-translation.cpp

Index: clang/test/Modules/cxx20-include-translation.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-include-translation.cpp
@@ -0,0 +1,109 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -xc++-user-header h1.h -emit-header-unit -o h1.pcm
+// RUN: %clang_cc1 -std=c++20 -xc++-user-header h2.h -emit-header-unit -o h2.pcm
+// RUN: %clang_cc1 -std=c++20 -xc++-user-header h3.h -emit-header-unit -o h3.pcm
+// RUN: %clang_cc1 -std=c++20 -xc++-user-header h4.h -emit-header-unit -o h4.pcm
+
+// RUN: %clang_cc1 -std=c++20 Xlate.cpp -emit-module-interface -o Xlate.pcm \
+// RUN: -fmodule-file=h1.pcm -fmodule-file=h2.pcm -fmodule-file=h3.pcm \
+// RUN: -fmodule-file=h4.pcm -fsyntax-only -Wauto-import -verify
+
+// Check that we do the intended translation and not more.
+// RUN: %clang_cc1 -std=c++20 Xlate.cpp \
+// RUN: -fmodule-file=h1.pcm -fmodule-file=h2.pcm -fmodule-file=h3.pcm \
+// RUN: -fmodule-file=h4.pcm  -E -undef | FileCheck %s
+
+// We expect no diagnostics here, the used functions should all be available.
+// RUN: %clang_cc1 -std=c++20 Xlate.cpp -emit-module-interface \
+// RUN: -fmodule-file=h1.pcm -fmodule-file=h2.pcm -fmodule-file=h3.pcm \
+// RUN: -fmodule-file=h4.pcm -fsyntax-only
+
+// The content of the headers is not terribly important, we just want to check
+// whether they are textually included or include-translated.
+//--- h1.h
+#ifndef H1_GUARD
+#define H1_GUARD
+
+#define ONE 1
+
+void foo();
+
+#endif // H1_GUARD
+
+//--- h2.h
+#ifndef H2_GUARD
+#define H2_GUARD
+
+#define TWO 2
+
+void bar();
+
+#endif // H2_GUARD
+
+//--- h3.h
+#ifndef H3_GUARD
+#define H3_GUARD
+
+#define THREE 3
+
+void baz();
+
+#endif // H3_GUARD
+
+//--- h4.h
+#ifndef H4_GUARD
+#define H4_GUARD
+
+#define FOUR 4
+
+void boo();
+
+#endif // H4_GUARD
+
+//--- h5.h
+#ifndef H5_GUARD
+#define H5_GUARD
+
+#define FIVE 5
+
+void five();
+
+#endif // H4_GUARD
+
+//--- Xlate.cpp
+/* some comment ...
+  ... */
+module /*nothing here*/;
+
+// This should be include-translated, when the header unit for h1 is available.
+#include "h1.h" // expected-warning {{treating #include as an import of module './h1.h'}}
+// Import of a header unit is allowed, named modules are not.
+import "h2.h";
+// A regular, untranslated, header
+#include "h5.h"
+
+export module Xlate;
+
+// This is OK, the import immediately follows the module decl.
+import "h3.h";
+
+// This should *not* be include-translated, even if header unit for h4 is
+// available.
+#include "h4.h"
+
+export void charlie() {
+  foo();
+  bar();
+  baz();
+  boo();
+  five();
+}
+
+// CHECK: #pragma clang module import "./h1.h"
+// CHECK: import ./h2.h
+// CHECK: import ./h3.h
+// CHECK-NOT: #pragma clang module import "./h4.h"
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -663,12 +663,23 @@
 return false;
   }
 
-  case tok::annot_module_include:
-Actions.ActOnModuleInclude(Tok.getLocation(),
-   reinterpret_cast(
-   Tok.getAnnotationValue()));
+  case tok::annot_module_include: {
+auto Loc = Tok.getLocation();
+Module *Mod = reinterpret_cast(Tok.getAnnotationValue());
+// FIXME: We need a better way to disambiguate C++ clang modules and
+// standard C++ modules.
+if (!getLangOpts().CPlusPlusModules || !Mod->isHeaderUnit() ||
+getLangOpts().ModulesTS)
+  Actions.ActOnModuleInclude(Loc, Mod);
+else {
+  DeclResult Import =
+  Actions.ActOnModuleImport(Loc, SourceLocation(), Loc, Mod);
+  Decl *ImportDecl = Import.isInvalid() ? nullptr : Import.get();
+  Result = Actions.ConvertDeclToDeclGroup(ImportDecl);
+}
 ConsumeAnnotationToken();
 return false;
+  }
 
   case tok::annot_module_begin:
 Actions.ActOnModuleBegin(Tok.getLocation(), reinterpret_cast(
Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -941,6 +941,9 @@
 
   // Update ImportSeqState to track our position within a C++20 import-seq
   // if this token is being produced as a result of phase 4 of translation.
+  // Update TrackGMFState to decide if we are currently in a Global Module
+  // Fragment. GMF state updates should 

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D128415#3628274 , @tschuett wrote:

> The Clang release notes indicate that PACBTI is off by default. In several 
> places, I can see PACBTI. Is the `ARM.td` missing something?

Nope, I should have re-checked the whole patch. Update coming to address this 
and @dmgreen's last comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

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

[PATCH] D128449: [clang] Introduce -Warray-parameter

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 442084.
serge-sans-paille added a comment.

Fix handling of `ConstantArrayType`, thanks @aaron.ballman for the hint.


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

https://reviews.llvm.org/D128449

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Misc/warning-wall.c
  clang/test/Sema/array-parameter.c
  clang/test/Sema/array-parameter.cpp

Index: clang/test/Sema/array-parameter.cpp
===
--- /dev/null
+++ clang/test/Sema/array-parameter.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -Warray-parameter -verify %s
+
+template 
+void func(int i[10]); // expected-note {{previously declared as 'int[10]' here}}
+
+template 
+void func(int i[N]); // expected-warning {{argument 'i' of type 'int[N]' with mismatched bound}}
+
+static constexpr int Extent = 10;
+void funk(int i[10]);
+void funk(int i[Extent]); // no-warning
Index: clang/test/Sema/array-parameter.c
===
--- /dev/null
+++ clang/test/Sema/array-parameter.c
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -Warray-parameter -verify %s
+void f0(int a[]);
+void f0(int *a); // no warning
+
+void f1(int a[]);  // expected-note {{previously declared as 'int[]' here}}
+void f1(int a[2]); // expected-warning {{argument 'a' of type 'int[2]' with mismatched bound}}
+
+void f2(int a[3]); // expected-note {{previously declared as 'int[3]' here}}
+void f2(int a[2]); // expected-warning {{argument 'a' of type 'int[2]' with mismatched bound}}
+
+void f3(int a[const 2]);
+void f3(int a[2]); // no warning
+
+void f4(int a[static 2]);
+void f4(int a[2]); // no warning
+
+void f5(int a[restrict 2]);
+void f5(int a[2]); // no warning
+
+void f6(int a[volatile 2]);
+void f6(int a[2]); // no warning
+
+void f7(int a[*]);
+void f7(int a[]); // no warning
+
+void f8(int n, int a[*]); // expected-note {{previously declared as 'int[*]' here}}
+void f8(int n, int a[n]); // expected-warning {{argument 'a' of type 'int[n]' with mismatched bound}}
+
+void f9(int *a);
+void f9(int a[2]);
+void f9(int a[]); // expected-warning {{argument 'a' of type 'int[]' with mismatched bound}}
+  // expected-note@-2 {{previously declared as 'int[2]' here}}
+void f9(int a[2]) // expected-warning {{argument 'a' of type 'int[2]' with mismatched bound}}
+  // expected-note@-3 {{previously declared as 'int[]' here}}
+{}
Index: clang/test/Misc/warning-wall.c
===
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -3,6 +3,7 @@
 
  CHECK:-Wall
 CHECK-NEXT:  -Wmost
+CHECK-NEXT:-Warray-parameter
 CHECK-NEXT:-Wbool-operation
 CHECK-NEXT:-Wbitwise-instead-of-logical
 CHECK-NEXT:-Wchar-subscripts
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -3209,6 +3209,36 @@
   if (!foundAny) newDecl->dropAttrs();
 }
 
+static bool EquivalentArrayTypes(QualType Old, QualType New, Sema ) {
+  auto NoSizeInfo = [](QualType Ty) {
+return Ty->isIncompleteArrayType() || Ty->isPointerType() ||
+   (Ty->isVariableArrayType() &&
+cast(Ty)->getSizeModifier() ==
+ArrayType::ArraySizeModifier::Star);
+  };
+
+  // `type[]` is equivalent to `type *` and `type[*]`.
+  if (NoSizeInfo(Old) && NoSizeInfo(New))
+return true;
+
+  // Don't try to compare VLA sizes, unless one of them has the star modifier.
+  if (Old->isVariableArrayType() && New->isVariableArrayType()) {
+const auto *OldVAT = cast(Old);
+const auto *NewVAT = cast(New);
+if ((OldVAT->getSizeModifier() == ArrayType::ArraySizeModifier::Star) ^
+(NewVAT->getSizeModifier() == ArrayType::ArraySizeModifier::Star))
+  return false;
+return true;
+  }
+
+  // Only compare size, ignore Size modifiers and CVR.
+  if (Old->isConstantArrayType() && New->isConstantArrayType())
+return S.getASTContext().getAsConstantArrayType(Old)->getSize() ==
+   S.getASTContext().getAsConstantArrayType(New)->getSize();
+
+  return Old == New;
+}
+
 static void mergeParamDeclTypes(ParmVarDecl *NewParam,
 const ParmVarDecl *OldParam,
 Sema ) {
@@ -3234,6 +3264,19 @@
   NewParam->setType(NewT);
 }
   }
+  const auto *OldParamDT = dyn_cast(OldParam->getType());
+  const auto *NewParamDT = dyn_cast(NewParam->getType());
+  if (OldParamDT && NewParamDT &&
+  OldParamDT->getPointeeType() == NewParamDT->getPointeeType()) {
+QualType OldParamOT = OldParamDT->getOriginalType();
+QualType NewParamOT = 

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Dave Green via Phabricator via cfe-commits]

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

Other than the release note change it might be worth adding some tests for 
-mcpu=cortex-m85+nopacbti and related configurations.

Otherwise LGTM




Comment at: clang/docs/ReleaseNotes.rst:541
+- clang now supports the Cortex-M85 CPU, which can be chosen with
+  `-mcpu=cortex-m85`. By default, this has PACBTI turned off, but it can be
+  enabled with `-mcpu=cortex-m85+pacbti`.

This can be updated now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

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

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

The Clang release notes indicate that PACBTI is off by default. In several 
places, I can see PACBTI. Is the `ARM.td` missing something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

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

[PATCH] D129074: [pseudo] Use the prebuilt cxx grammar for the lit tests, NFC.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D129074#3628221 , @sammccall wrote:

> Can we make this the default value of the flag instead?

Yeah, the default one is the `cxx`. But I found it is a bit clearer to 
explicitly mention it in the code. (also happy to remove all 
`-grammar=%cxx-bnf-file`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129074

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

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Sam Elliott via Phabricator via cfe-commits]

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

The other changes in this commit are to enable PACBTI by default, as agreed 
with stakeholders within Arm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

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

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 442079.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/CodeGen/arm-target-features.c
  clang/test/Driver/arm-cortex-cpus-2.c
  clang/test/Driver/arm-nofp-disabled-features.c
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Target/ARM/ARM.td
  llvm/test/CodeGen/ARM/build-attributes.ll
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -395,13 +395,19 @@
  ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
  ARM::AEK_FP16,
  "8.1-M.Mainline"),
+ARMCPUTestParams("cortex-m85", "armv8.1-m.main",
+ "fp-armv8-fullfp16-d16",
+ ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_SIMD |
+ ARM::AEK_FP | ARM::AEK_RAS | ARM::AEK_LOB |
+ ARM::AEK_FP16 | ARM::AEK_PACBTI,
+ "8.1-M.Mainline"),
 ARMCPUTestParams("iwmmxt", "iwmmxt", "none", ARM::AEK_NONE, "iwmmxt"),
 ARMCPUTestParams("xscale", "xscale", "none", ARM::AEK_NONE, "xscale"),
 ARMCPUTestParams("swift", "armv7s", "neon-vfpv4",
  ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP,
  "7-S")));
 
-static constexpr unsigned NumARMCPUArchs = 88;
+static constexpr unsigned NumARMCPUArchs = 89;
 
 TEST(TargetParserTest, testARMCPUArchList) {
   SmallVector List;
Index: llvm/test/CodeGen/ARM/build-attributes.ll
===
--- llvm/test/CodeGen/ARM/build-attributes.ll
+++ llvm/test/CodeGen/ARM/build-attributes.ll
@@ -235,6 +235,7 @@
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+mve.fp | FileCheck %s --check-prefix=ARMv81M-MAIN-MVEFP
 ; RUN: llc < %s -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+pacbti | FileCheck %s --check-prefix=ARMv81M-MAIN-PACBTI
 ; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m55 | FileCheck %s --check-prefix=CORTEX-M55
+; RUN: llc < %s -mtriple=arm-none-none-eabi -mcpu=cortex-m85 | FileCheck %s --check-prefix=CORTEX-M85
 
 ; CPU-SUPPORTED-NOT: is not a recognized processor for this target
 
@@ -1748,6 +1749,19 @@
 ; CORTEX-M55: .eabi_attribute 38, 1
 ; CORTEX-M55: .eabi_attribute 14, 0
 
+; CORTEX-M85: .cpu cortex-m85
+; CORTEX-M85: .eabi_attribute 6, 21   @ Tag_CPU_arch
+; CORTEX-M85: .eabi_attribute 7, 77   @ Tag_CPU_arch_profile
+; CORTEX-M85: .eabi_attribute 8, 0@ Tag_ARM_ISA_use
+; CORTEX-M85: .eabi_attribute 9, 3@ Tag_THUMB_ISA_use
+; CORTEX-M85: .fpufpv5-d16
+; CORTEX-M85: .eabi_attribute 36, 1   @ Tag_FP_HP_extension
+; CORTEX-M85: .eabi_attribute 48, 2   @ Tag_MVE_arch
+; CORTEX-M85: .eabi_attribute 46, 1   @ Tag_DSP_extension
+; CORTEX-M85: .eabi_attribute 34, 1   @ Tag_CPU_unaligned_access
+; CORTEX-M85-NOT: .eabi_attribute 50
+; CORTEX-M85-NOT: .eabi_attribute 52
+
 define i32 @f(i64 %z) {
 ret i32 0
 }
Index: llvm/lib/Target/ARM/ARM.td
===
--- llvm/lib/Target/ARM/ARM.td
+++ llvm/lib/Target/ARM/ARM.td
@@ -1450,6 +1450,12 @@
  HasMVEFloatOps,
  FeatureFixCMSE_CVE_2021_35465]>;
 
+def : ProcessorModel<"cortex-m85", CortexM7Model,   [ARMv81mMainline,
+ FeatureDSP,
+ FeatureFPARMv8_D16,
+ FeatureUseMISched,
+ HasMVEFloatOps]>;
+
 def : ProcNoItin<"cortex-a32",   [ARMv8a,
  FeatureHWDivThumb,
  FeatureHWDivARM,
Index: llvm/include/llvm/Support/ARMTargetParser.def
===
--- llvm/include/llvm/Support/ARMTargetParser.def
+++ llvm/include/llvm/Support/ARMTargetParser.def
@@ -303,6 +303,9 @@
 ARM_CPU_NAME("cortex-m35p", ARMV8MMainline, FK_FPV5_SP_D16, false, ARM::AEK_DSP)
 ARM_CPU_NAME("cortex-m55", ARMV8_1MMainline, FK_FP_ARMV8_FULLFP16_D16, false,
  (ARM::AEK_DSP | ARM::AEK_SIMD | ARM::AEK_FP | ARM::AEK_FP16))
+ARM_CPU_NAME("cortex-m85", ARMV8_1MMainline, FK_FP_ARMV8_FULLFP16_D16, false,
+ (ARM::AEK_DSP | ARM::AEK_SIMD | ARM::AEK_FP | ARM::AEK_FP16 |
+  ARM::AEK_RAS | 

[PATCH] D128485: [pseudo] Store shift and goto actions in a compact structure with faster lookup.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h:72
   // action types, this class becomes less useful. Remove it.
   class Action {
   public:

The Action can be removed now (I guess your plan is to remove it in a follow-up 
patch).



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h:128
   // Expected to be called by LR parsers.
   // REQUIRES: Nonterminal is valid here.
+  llvm::Optional getGoToState(StateID State,

If we're going to change the return type to `llvm::Optional`.

I think we can remove this comment , as the the invariant is not guaranteed by 
the API now, but rather than moving to the caller.



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h:131
+   SymbolID Nonterminal) const {
+return Gotos.get(numStates() * Nonterminal + State);
+  }

add nonterminal assertion?



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h:213
+  // Lookup is constant time with a low factor (no hashing).
+  class SparseTable {
+using Word = uint64_t;

As discussed offline, we could be more specific for the name (something like 
`StateIDTable`).

We might also want to put the key calculation logic (`numStates() * Nonterminal 
+ State`, and `numStates() * symbolToToken(Terminal) + State`) to an inline 
function, we have a few places using them duplicately. 



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h:223
+SparseTable() = default;
+SparseTable(const llvm::DenseMap , unsigned NumKeys) {
+  assert(

nit: V => KV
it feels more natural to make it a static `build` method, but up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128485

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

[PATCH] D129074: [pseudo] Use the prebuilt cxx grammar for the lit tests, NFC.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Can we make this the default value of the flag instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129074

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

[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

[Iain Sandoe via Phabricator via cfe-commits]

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

In D129045#3627901 , @ChuanqiXu wrote:

> In D129045#3627878 , @iains wrote:
>
>> In D129045#3627856 , @ChuanqiXu 
>> wrote:
>>
>>> It looks like the tests lack the cast that the methods are attached to the 
>>> global module fragment.
>>
>> (maybe I misunderstand what you are saying here)



> I mean the tests instead of the implementation. It looks like there is no GMF 
> in the tests.

added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129045

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

[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

[Iain Sandoe via Phabricator via cfe-commits]

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

updated testcases to fix a missing new line and add included header in a GMF.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129045

Files:
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/AST/ast-dump-constant-expr.cpp
  clang/test/AST/ast-dump-lambda.cpp
  clang/test/CXX/class/class.friend/p7-cxx20.cpp
  clang/test/CXX/class/class.mfct/p1-cxx20.cpp

Index: clang/test/CXX/class/class.mfct/p1-cxx20.cpp
===
--- /dev/null
+++ clang/test/CXX/class/class.mfct/p1-cxx20.cpp
@@ -0,0 +1,57 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 no-modules.cpp -fsyntax-only -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-NM %s
+// RUN: %clang_cc1 -std=c++20 -xc++-user-header header-unit.h -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-HU %s
+// RUN: %clang_cc1 -std=c++20 module.cpp -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-MOD %s
+
+//--- no-modules.cpp
+
+class X {
+  void x(){};
+};
+
+// CHECK-NM: `-CXXRecordDecl {{.*}}  line:2:7 class X definition
+// CHECK-NM:   |-CXXRecordDecl {{.*}}  col:7 implicit class X
+// CHECK-NM-NEXT: `-CXXMethodDecl {{.*}}  col:8 x 'void ()' implicit-inline
+
+// A header unit header
+//--- header-unit.h
+
+class Y {
+  void y(){};
+};
+
+// CHECK-HU: `-CXXRecordDecl {{.*}} <./header-unit.h:2:1, line:4:1> line:2:7 class Y definition
+// CHECK-HU: |-CXXRecordDecl {{.*}}  col:7 implicit class Y
+// CHECK-HU-NEXT: `-CXXMethodDecl {{.*}}  col:8 y 'void ()' implicit-inline
+
+// A textually-included header
+//--- header.h
+
+class A {
+  void a(){};
+};
+
+//--- module.cpp
+module;
+#include "header.h"
+
+export module M;
+
+class Z {
+  void z(){};
+};
+
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <./header.h:2:1, line:4:1> line:2:7 in M. hidden class A definition
+// CHECK-MOD: | |-CXXRecordDecl {{.*}}  col:7 in M. hidden implicit class A
+// CHECK-MOD-NEXT: | `-CXXMethodDecl {{.*}}  col:8 in M. hidden a 'void ()' implicit-inline
+
+// CHECK-MOD: `-CXXRecordDecl {{.*}}  line:6:7 in M hidden class Z{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: |-CXXRecordDecl {{.*}}  col:7 in M hidden implicit class Z{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: `-CXXMethodDecl {{.*}}  col:8 in M hidden z 'void ()'{{( ReachableWhenImported)?}}
Index: clang/test/CXX/class/class.friend/p7-cxx20.cpp
===
--- /dev/null
+++ clang/test/CXX/class/class.friend/p7-cxx20.cpp
@@ -0,0 +1,59 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 no-modules.cpp -fsyntax-only -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-NM %s
+// RUN: %clang_cc1 -std=c++20 -xc++-user-header header-unit.h -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-HU %s
+// RUN: %clang_cc1 -std=c++20 module.cpp -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-MOD %s
+
+//--- no-modules.cpp
+
+class X {
+  friend void x(){};
+};
+
+// CHECK-NM: `-CXXRecordDecl {{.*}}  line:2:7 class X definition
+// CHECK-NM:   |-CXXRecordDecl {{.*}}  col:7 implicit class X
+// CHECK-NM-NEXT: `-FriendDecl {{.*}}  col:15
+// CHECK-NM-NEXT: `-FunctionDecl {{.*}} parent {{.*}}  col:15 x 'void ()' implicit-inline
+
+//--- header-unit.h
+
+class Y {
+  friend void y(){};
+};
+
+// CHECK-HU: `-CXXRecordDecl {{.*}} <./header-unit.h:2:1, line:4:1> line:2:7 class Y definition
+// CHECK-HU: |-CXXRecordDecl {{.*}}  col:7 implicit class Y
+// CHECK-HU-NEXT: `-FriendDecl {{.*}}  col:15
+// CHECK-HU-NEXT: `-FunctionDecl {{.*}} parent {{.*}}  col:15 y 'void ()' implicit-inline
+
+// A textually-included header
+//--- header.h
+
+class A {
+  friend void a(){};
+};
+
+//--- module.cpp
+module;
+#include "header.h"
+
+export module M;
+
+class Z {
+  friend void z(){};
+};
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <./header.h:2:1, line:4:1> line:2:7 in M. hidden class A definition
+// CHECK-MOD: | |-CXXRecordDecl {{.*}}  col:7 in M. hidden implicit class A
+// CHECK-MOD-NEXT: | `-FriendDecl {{.*}}  col:15 in M.
+// CHECK-MOD-NEXT: |   `-FunctionDecl {{.*}} parent {{.*}}  col:15 in M. hidden a 'void ()' implicit-inline
+
+// CHECK-MOD: `-CXXRecordDecl {{.*}}  line:6:7 in M hidden class Z{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: |-CXXRecordDecl {{.*}}  col:7 in M hidden implicit class Z{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: `-FriendDecl {{.*}}  col:15 in M{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: `-FunctionDecl {{.*}} parent {{.*}}  col:15 in M hidden z 'void ()'{{( ReachableWhenImported)?}}
Index: clang/test/AST/ast-dump-lambda.cpp

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

In D107082#3628120 , @sylvestre.ledru 
wrote:

> @pengfei I am not convinced it is an issue on my side. I don't have anything 
> particular in this area and using a stage2 build system.
>
> Anyway, this patch fixes the issue on my side:
> https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/snapshot/debian/patches/force-sse2-compiler-rt.diff

I don't have much experience in compiler-rt and multi stage build. So I may be 
wrong. It looks to me like an existing problem just exposed by this patch. The 
diff is another proof.
The build command tells us it's a 32-bit build. But the change for `x86_64` 
solves it, which confirms my previous guess: You are using one configure for 
CMake (probobally 64 bit) but build for 32 bit target.
Although the diff works, it doesn't look a clean solution to me. But I don't 
have better suggestion either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107082

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

[PATCH] D129074: [pseudo] Use the prebuilt cxx grammar for the lit tests, NFC.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: alextsao1999.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129074

Files:
  clang-tools-extra/pseudo/test/cxx/capture-list.cpp
  clang-tools-extra/pseudo/test/cxx/declarator-function.cpp
  clang-tools-extra/pseudo/test/cxx/declarator-var.cpp
  clang-tools-extra/pseudo/test/cxx/empty-member-spec.cpp
  clang-tools-extra/pseudo/test/cxx/keyword.cpp
  clang-tools-extra/pseudo/test/cxx/mixed-designator.cpp
  clang-tools-extra/pseudo/test/cxx/parameter-decl-clause.cpp
  clang-tools-extra/pseudo/test/cxx/predefined-identifier.cpp
  clang-tools-extra/pseudo/test/cxx/template-empty-type-parameter.cpp
  clang-tools-extra/pseudo/test/cxx/unsized-array.cpp

Index: clang-tools-extra/pseudo/test/cxx/unsized-array.cpp
===
--- clang-tools-extra/pseudo/test/cxx/unsized-array.cpp
+++ clang-tools-extra/pseudo/test/cxx/unsized-array.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-pseudo -grammar=%cxx-bnf-file -source=%s --print-forest | FileCheck %s
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
 void s(int[]);
 // CHECK:  parameter-declaration-clause~parameter-declaration := decl-specifier-seq abstract-declarator
 // CHECK-NEXT: ├─decl-specifier-seq~INT := tok[3]
Index: clang-tools-extra/pseudo/test/cxx/template-empty-type-parameter.cpp
===
--- clang-tools-extra/pseudo/test/cxx/template-empty-type-parameter.cpp
+++ clang-tools-extra/pseudo/test/cxx/template-empty-type-parameter.cpp
@@ -1,3 +1,3 @@
-// RUN: clang-pseudo -grammar=%cxx-bnf-file -source=%s --print-forest | FileCheck %s
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
 template  struct MatchParents;
 // CHECK: template-parameter-list~TYPENAME := tok[2]
Index: clang-tools-extra/pseudo/test/cxx/predefined-identifier.cpp
===
--- clang-tools-extra/pseudo/test/cxx/predefined-identifier.cpp
+++ clang-tools-extra/pseudo/test/cxx/predefined-identifier.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-pseudo -grammar=%cxx-bnf-file -source=%s --print-forest | FileCheck %s
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
 void s() {
   __func__;
   // CHECK: expression~__FUNC__ := tok[5]
Index: clang-tools-extra/pseudo/test/cxx/parameter-decl-clause.cpp
===
--- clang-tools-extra/pseudo/test/cxx/parameter-decl-clause.cpp
+++ clang-tools-extra/pseudo/test/cxx/parameter-decl-clause.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-pseudo -grammar=%cxx-bnf-file -source=%s --print-forest | FileCheck %s
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
 void foo2(int, ...);
 // CHECK:  translation-unit~simple-declaration := decl-specifier-seq init-declarator-list ;
 // CHECK-NEXT: ├─decl-specifier-seq~VOID :=
Index: clang-tools-extra/pseudo/test/cxx/mixed-designator.cpp
===
--- clang-tools-extra/pseudo/test/cxx/mixed-designator.cpp
+++ clang-tools-extra/pseudo/test/cxx/mixed-designator.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-pseudo -grammar=%cxx-bnf-file -source=%s --print-forest | FileCheck %s
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
 // FIXME: tighten CHECK to CHECK-NEXT once numeric literals are unambiguous.
 auto x = { 1, .f = 2, [c]{3} };
 // CHECK:  initializer-clause~braced-init-list
Index: clang-tools-extra/pseudo/test/cxx/keyword.cpp
===
--- clang-tools-extra/pseudo/test/cxx/keyword.cpp
+++ clang-tools-extra/pseudo/test/cxx/keyword.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-pseudo -grammar=%cxx-bnf-file -source=%s --print-forest | FileCheck %s
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
 bool operator<();
 // CHECK:  translation-unit~simple-declaration := decl-specifier-seq init-declarator-list ;
 // CHECK-NEXT: ├─decl-specifier-seq~BOOL
Index: clang-tools-extra/pseudo/test/cxx/empty-member-spec.cpp
===
--- clang-tools-extra/pseudo/test/cxx/empty-member-spec.cpp
+++ clang-tools-extra/pseudo/test/cxx/empty-member-spec.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-pseudo -grammar=%cxx-bnf-file -source=%s --print-forest | FileCheck %s
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
 class Foo {
 public:
 };
Index: clang-tools-extra/pseudo/test/cxx/declarator-var.cpp
===
--- clang-tools-extra/pseudo/test/cxx/declarator-var.cpp
+++ clang-tools-extra/pseudo/test/cxx/declarator-var.cpp
@@ 

[PATCH] D129065: [RISCV][Driver] Add libm linking to `RISCVToolchain`

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 442069.
anton-afanasyev added a comment.

Update summary


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129065

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c


Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -58,7 +58,7 @@
 // CXX-RV64-BAREMETAL-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV64-BAREMETAL-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
 // CXX-RV64-BAREMETAL-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
-// CXX-RV64-BAREMETAL-LP64: "-lstdc++" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
+// CXX-RV64-BAREMETAL-LP64: "-lstdc++" "-lm" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
 // CXX-RV64-BAREMETAL-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clangxx -### %s -fuse-ld= \
@@ -73,7 +73,7 @@
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv64-unknown-elf/lib"
-// CXX-RV64-BAREMETAL-NOSYSROOT-LP64: "-lstdc++" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
+// CXX-RV64-BAREMETAL-NOSYSROOT-LP64: "-lstdc++" "-lm" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clang -### %s -fuse-ld= -no-pie \
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -58,7 +58,7 @@
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV32-BAREMETAL-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1"
 // CXX-RV32-BAREMETAL-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib"
-// CXX-RV32-BAREMETAL-ILP32: "-lstdc++" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
+// CXX-RV32-BAREMETAL-ILP32: "-lstdc++" "-lm" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: %clangxx -### %s -fuse-ld= \
@@ -73,7 +73,7 @@
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv32-unknown-elf/lib"
-// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-lstdc++" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-lstdc++" "-lm" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clang -### %s -fuse-ld=ld -no-pie \
Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -211,8 +211,11 @@
 
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs)) {
-if (ToolChain.ShouldLinkCXXStdlib(Args))
-  ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
+if (D.CCCIsCXX()) {
+  if (ToolChain.ShouldLinkCXXStdlib(Args))
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
+  CmdArgs.push_back("-lm");
+}
 CmdArgs.push_back("--start-group");
 CmdArgs.push_back("-lc");
 CmdArgs.push_back("-lgloss");


Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -58,7 +58,7 @@
 // CXX-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
 // CXX-RV64-BAREMETAL-LP64: 

[PATCH] D129057: [clang-format] Break on AfterColon only if not followed by comment

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

LGTM


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

https://reviews.llvm.org/D129057

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

[PATCH] D129064: [clang-format] Avoid crash in LevelIndentTracker.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129064

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

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

@pengfei I am not convinced it is an issue on my side. I don't have anything 
particular in this area and using a stage2 build system.

Anyway, this patch fixes the issue on my side:
https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/snapshot/debian/patches/force-sse2-compiler-rt.diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107082

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

[PATCH] D127874: [analyzer] Reimplement UnreachableCodeChecker using worklists

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> I don't think any of the issues mentioned in this patch relates to strongly 
> connected components, thus I don't think I can answer to this question.

In your example above, repeated here:

  #6(entry)  #2(goto a;)
   |  |^
  #5(goto end;)   | \
   | #4(goto b;) |
  #1(end:)|  |
   | #3(goto c;) |
  #0(exit) \/

[#2, #4, #3] is a strongly connected (and unreachable) component  of the CFG, 
isn't it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127874

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

[PATCH] D128535: [analyzer] Improve loads from reinterpret-cast fields

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

I like it, but I'd like to know more about the nature of the applied "hack" and 
if there is a way to solve it properly in the longer term.




Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2014-2017
+  // FIXME: This is a hack, and doesn't do anything really intelligent yet.
+  // FIXME: This hack is analogous with the one present in
+  // `getBindingForElement`, maybe we should handle both in
+  // `getBindingForFieldOrElementCommon`.

Could you please elaborate why this is a hack? What should be done and where to 
solve it properly? (Disclaimer, I did not look into 
`getBindingForFieldOrElementCommon`.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128535

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

[PATCH] D129070: [clang-tidy] Fixed an issue in readability-identifier-naming not using options specified.

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, LegalizeAdulthood.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
njames93 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Fixed an issue where specifying empty strings for prefix or suffix would be 
ignored preventing using those to override a different style.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129070

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-empty-options.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-empty-options.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-empty-options.cpp
@@ -0,0 +1,13 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: {\
+// RUN: readability-identifier-naming.GlobalConstantPrefix: "", \
+// RUN: readability-identifier-naming.GlobalVariablePrefix: g_ \
+// RUN:  }}'
+
+int BadGlobalVariable;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'BadGlobalVariable' [readability-identifier-naming]
+// CHECK-FIXES: int g_BadGlobalVariable;
+int g_GoodGlobalVariable;
+
+const int GoodGlobalConstant = 0;
+const int g_IgnoreGlobalConstant = 0;
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -217,6 +217,11 @@
 - Fixed incorrect suggestions for :doc:`readability-container-size-empty
   ` when smart pointers are involved.
 
+- Fixed an issue in :doc:`readability-identifier-naming
+  ` when specifying an empty
+  string for ``Prefix`` or ``Suffix`` options could result in the style not
+  being used.
+
 - Fixed a false positive in :doc:`readability-non-const-parameter
   ` when the parameter is
   referenced by an lvalue.
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -69,20 +69,20 @@
   struct NamingStyle {
 NamingStyle() = default;
 
-NamingStyle(llvm::Optional Case, const std::string ,
-const std::string , const std::string ,
+NamingStyle(llvm::Optional Case, StringRef Prefix,
+StringRef Suffix, StringRef IgnoredRegexpStr,
 HungarianPrefixType HPType);
 NamingStyle(const NamingStyle ) = delete;
 NamingStyle =(NamingStyle &) = default;
 NamingStyle(NamingStyle &) = default;
 
 llvm::Optional Case;
-std::string Prefix;
-std::string Suffix;
+StringRef Prefix;
+StringRef Suffix;
 // Store both compiled and non-compiled forms so original value can be
 // serialized
 llvm::Regex IgnoredRegexp;
-std::string IgnoredRegexpStr;
+StringRef IgnoredRegexpStr;
 
 HungarianPrefixType HPType;
   };
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -228,9 +228,8 @@
 // clang-format on
 
 IdentifierNamingCheck::NamingStyle::NamingStyle(
-llvm::Optional Case,
-const std::string , const std::string ,
-const std::string , HungarianPrefixType HPType)
+llvm::Optional Case, StringRef Prefix,
+StringRef Suffix, StringRef IgnoredRegexpStr, HungarianPrefixType HPType)
 : Case(Case), Prefix(Prefix), Suffix(Suffix),
   IgnoredRegexpStr(IgnoredRegexpStr), HPType(HPType) {
   if (!IgnoredRegexpStr.empty()) {
@@ -263,22 +262,21 @@
 
 memcpy([StyleSize], "IgnoredRegexp", 13);
 StyleString.truncate(StyleSize + 13);
-StringRef IgnoredRegexpStr = Options.get(StyleString, "");
+Optional IgnoredRegexpStr = Options.get(StyleString);
 memcpy([StyleSize], "Prefix", 6);
 StyleString.truncate(StyleSize + 6);
-std::string Prefix(Options.get(StyleString, ""));
+Optional Prefix(Options.get(StyleString));
 // Fast replacement of [Pre]fix -> [Suf]fix.
 memcpy([StyleSize], "Suf", 3);
-std::string Postfix(Options.get(StyleString, ""));
+Optional Postfix(Options.get(StyleString));
 memcpy([StyleSize], "Case", 4);
 StyleString.pop_back_n(2);
-auto CaseOptional =
+llvm::Optional 

[PATCH] D128608: [NFC][ASTImporter] remove the unnecessary condition checks in ASTImporter.cpp

[Gabor Marton via Phabricator via cfe-commits]

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

LGTM

(I don't quite understand the rename and the [LLDB] tag, this is clearly in 
[Clang][ASTImporter]. Yes, LLDB depends on the ASTImporter).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128608

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

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

[Gabor Marton via Phabricator via cfe-commits]

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

Thanks for the update! LGTM (with very minor naming nits).




Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:146
+
+static bool HandleAST(StringRef astPath) {
+





Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:160
+
+  FileManager fm(CI->getFileSystemOpts());
+  SmallString<128> absPath(astPath);





Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:171-172
+
+static int HandleFiles(ArrayRef sourceFiles,
+   CompilationDatabase ) {
+  std::vector SourcesToBeParsed;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128704

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

[PATCH] D129068: [AST] Profiling on constraint expression instead of arguments for TypeConstraint in ASTContext::isSameTemplateParameter

[Chuanqi Xu via Phabricator via cfe-commits]

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

Remove unnecessary changes.


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

https://reviews.llvm.org/D129068

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/Modules/concept.cppm


Index: clang/test/Modules/concept.cppm
===
--- clang/test/Modules/concept.cppm
+++ clang/test/Modules/concept.cppm
@@ -18,6 +18,9 @@
 template 
 concept __member_size = requires(_Tp &) { t.size(); };
 
+template 
+concept C = requires(First x, Second y) { x+y; };
+
 struct A {
 public:
   template 
@@ -29,6 +32,11 @@
   constexpr __integer_like auto operator()(_Tp&& __t) const {
 return __t.size();
   }
+
+  template <__integer_like _Tp, C<_Tp> Sentinel>
+  constexpr _Tp operator()(_Tp &&__t, Sentinel &) const {
+return __t;
+  }
 };
 #endif
 
@@ -51,4 +59,5 @@
 auto operator+(S s) { return 0; }
 };
 __fn{}(S());
+__fn{}(S(), S());
 }
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6245,11 +6245,14 @@
 auto *TYTCArgs = TYTC->getTemplateArgsAsWritten();
 if (TXTCArgs->NumTemplateArgs != TYTCArgs->NumTemplateArgs)
   return false;
+
 llvm::FoldingSetNodeID XID, YID;
-for (auto  : TXTCArgs->arguments())
-  ArgLoc.getArgument().Profile(XID, X->getASTContext());
-for (auto  : TYTCArgs->arguments())
-  ArgLoc.getArgument().Profile(YID, Y->getASTContext());
+auto *XConstraint = TXTC->getImmediatelyDeclaredConstraint();
+auto *YConstraint = TYTC->getImmediatelyDeclaredConstraint();
+if (XConstraint)
+  XConstraint->Profile(XID, *this, /*Canonical=*/true);
+if (YConstraint)
+  YConstraint->Profile(YID, *this, /*Canonical=*/true);
 if (XID != YID)
   return false;
   }


Index: clang/test/Modules/concept.cppm
===
--- clang/test/Modules/concept.cppm
+++ clang/test/Modules/concept.cppm
@@ -18,6 +18,9 @@
 template 
 concept __member_size = requires(_Tp &) { t.size(); };
 
+template 
+concept C = requires(First x, Second y) { x+y; };
+
 struct A {
 public:
   template 
@@ -29,6 +32,11 @@
   constexpr __integer_like auto operator()(_Tp&& __t) const {
 return __t.size();
   }
+
+  template <__integer_like _Tp, C<_Tp> Sentinel>
+  constexpr _Tp operator()(_Tp &&__t, Sentinel &) const {
+return __t;
+  }
 };
 #endif
 
@@ -51,4 +59,5 @@
 auto operator+(S s) { return 0; }
 };
 __fn{}(S());
+__fn{}(S(), S());
 }
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6245,11 +6245,14 @@
 auto *TYTCArgs = TYTC->getTemplateArgsAsWritten();
 if (TXTCArgs->NumTemplateArgs != TYTCArgs->NumTemplateArgs)
   return false;
+
 llvm::FoldingSetNodeID XID, YID;
-for (auto  : TXTCArgs->arguments())
-  ArgLoc.getArgument().Profile(XID, X->getASTContext());
-for (auto  : TYTCArgs->arguments())
-  ArgLoc.getArgument().Profile(YID, Y->getASTContext());
+auto *XConstraint = TXTC->getImmediatelyDeclaredConstraint();
+auto *YConstraint = TYTC->getImmediatelyDeclaredConstraint();
+if (XConstraint)
+  XConstraint->Profile(XID, *this, /*Canonical=*/true);
+if (YConstraint)
+  YConstraint->Profile(YID, *this, /*Canonical=*/true);
 if (XID != YID)
   return false;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129068: [AST] Profiling on constraint expression instead of arguments for TypeConstraint in ASTContext::isSameTemplateParameter

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu created this revision.
ChuanqiXu added reviewers: rsmith, vsapsai, ilya-biryukov.
ChuanqiXu added a project: clang-modules.
Herald added a project: All.
ChuanqiXu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The current implementation to judge the similarity of TypeConstraint in 
ASTContext::isSameTemplateParameter is problematic, it couldn't handle the 
following case:

  C++
  template <__integer_like _Tp, C<_Tp> Sentinel>
  constexpr _Tp operator()(_Tp &&__t, Sentinel &) const {
  return __t;
  }

When we see 2 such declarations from different modules, we would judge their 
similarity by `ASTContext::isSame*` methods. But problems come for the 
TypeConstraint. Originally, we would profile each argument one by one. But it 
is not right. Since the profiling result of `_Tp` would refer to two different 
template type declarations. So it would get different results. It is right 
since the `_Tp` in different modules refers to different declarations indeed. 
So the same declaration in different modules would meet incorrect our-checking 
results.

It is not the thing we want. We want to know if the TypeConstraint have the 
same expression.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129068

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/Modules/concept.cppm


Index: clang/test/Modules/concept.cppm
===
--- clang/test/Modules/concept.cppm
+++ clang/test/Modules/concept.cppm
@@ -18,6 +18,9 @@
 template 
 concept __member_size = requires(_Tp &) { t.size(); };
 
+template 
+concept C = requires(First x, Second y) { x+y; };
+
 struct A {
 public:
   template 
@@ -29,6 +32,11 @@
   constexpr __integer_like auto operator()(_Tp&& __t) const {
 return __t.size();
   }
+
+  template <__integer_like _Tp, C<_Tp> Sentinel>
+  constexpr _Tp operator()(_Tp &&__t, Sentinel &) const {
+return __t;
+  }
 };
 #endif
 
@@ -51,4 +59,5 @@
 auto operator+(S s) { return 0; }
 };
 __fn{}(S());
+__fn{}(S(), S());
 }
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6245,11 +6245,14 @@
 auto *TYTCArgs = TYTC->getTemplateArgsAsWritten();
 if (TXTCArgs->NumTemplateArgs != TYTCArgs->NumTemplateArgs)
   return false;
+
 llvm::FoldingSetNodeID XID, YID;
-for (auto  : TXTCArgs->arguments())
-  ArgLoc.getArgument().Profile(XID, X->getASTContext());
-for (auto  : TYTCArgs->arguments())
-  ArgLoc.getArgument().Profile(YID, Y->getASTContext());
+auto *XConstraint = TXTC->getImmediatelyDeclaredConstraint();
+auto *YConstraint = TYTC->getImmediatelyDeclaredConstraint();
+if (XConstraint)
+  XConstraint->Profile(XID, *this, /*Canonical=*/true);
+if (YConstraint)
+  YConstraint->Profile(YID, *this, /*Canonical=*/true);
 if (XID != YID)
   return false;
   }
@@ -6524,10 +6527,10 @@
   // and patterns match.
   if (const auto *TemplateX = dyn_cast(X)) {
 const auto *TemplateY = cast(Y);
-return isSameEntity(TemplateX->getTemplatedDecl(),
-TemplateY->getTemplatedDecl()) &&
-   isSameTemplateParameterList(TemplateX->getTemplateParameters(),
-   TemplateY->getTemplateParameters());
+return  isSameEntity(TemplateX->getTemplatedDecl(),
+ TemplateY->getTemplatedDecl()) &&
+isSameTemplateParameterList(TemplateX->getTemplateParameters(),
+TemplateY->getTemplateParameters());
   }
 
   // Fields with the same name and the same type match.


Index: clang/test/Modules/concept.cppm
===
--- clang/test/Modules/concept.cppm
+++ clang/test/Modules/concept.cppm
@@ -18,6 +18,9 @@
 template 
 concept __member_size = requires(_Tp &) { t.size(); };
 
+template 
+concept C = requires(First x, Second y) { x+y; };
+
 struct A {
 public:
   template 
@@ -29,6 +32,11 @@
   constexpr __integer_like auto operator()(_Tp&& __t) const {
 return __t.size();
   }
+
+  template <__integer_like _Tp, C<_Tp> Sentinel>
+  constexpr _Tp operator()(_Tp &&__t, Sentinel &) const {
+return __t;
+  }
 };
 #endif
 
@@ -51,4 +59,5 @@
 auto operator+(S s) { return 0; }
 };
 __fn{}(S());
+__fn{}(S(), S());
 }
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6245,11 +6245,14 @@
 auto *TYTCArgs = TYTC->getTemplateArgsAsWritten();
 if (TXTCArgs->NumTemplateArgs != TYTCArgs->NumTemplateArgs)
   return false;
+
 llvm::FoldingSetNodeID XID, YID;
-for (auto  

[PATCH] D129061: [Lex] Diagnose macro in command lines

[Jack Huang via Phabricator via cfe-commits]

jackhong12 added a comment.

Sorry, I didn't consider some cases. I'll fix it soon!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129061

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

[PATCH] D129065: [RISCV][Driver] Add libm linking to `RISCVToolchain`

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev created this revision.
anton-afanasyev added reviewers: MaskRay, kito-cheng, asb.
Herald added subscribers: sunshaoce, VincentWu, luke957, StephenFan, vkmr, 
frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, 
benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, 
edward-jones, zzheng, jrtc27, shiva0217, niosHD, sabuasal, simoncook, 
johnrusso, rbar, arichardson.
Herald added a project: All.
anton-afanasyev requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD.
Herald added a project: clang.

GCC automatically links libm by adding `-lm` to linker command line,
add it to `RISCVToochain` as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129065

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c


Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -58,7 +58,7 @@
 // CXX-RV64-BAREMETAL-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV64-BAREMETAL-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
 // CXX-RV64-BAREMETAL-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
-// CXX-RV64-BAREMETAL-LP64: "-lstdc++" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
+// CXX-RV64-BAREMETAL-LP64: "-lstdc++" "-lm" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
 // CXX-RV64-BAREMETAL-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clangxx -### %s -fuse-ld= \
@@ -73,7 +73,7 @@
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv64-unknown-elf/lib"
-// CXX-RV64-BAREMETAL-NOSYSROOT-LP64: "-lstdc++" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
+// CXX-RV64-BAREMETAL-NOSYSROOT-LP64: "-lstdc++" "-lm" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clang -### %s -fuse-ld= -no-pie \
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -58,7 +58,7 @@
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV32-BAREMETAL-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1"
 // CXX-RV32-BAREMETAL-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib"
-// CXX-RV32-BAREMETAL-ILP32: "-lstdc++" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
+// CXX-RV32-BAREMETAL-ILP32: "-lstdc++" "-lm" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: %clangxx -### %s -fuse-ld= \
@@ -73,7 +73,7 @@
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv32-unknown-elf/lib"
-// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-lstdc++" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-lstdc++" "-lm" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clang -### %s -fuse-ld=ld -no-pie \
Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -211,8 +211,11 @@
 
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs)) {
-if (ToolChain.ShouldLinkCXXStdlib(Args))
-  ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
+if (D.CCCIsCXX()) {
+  if (ToolChain.ShouldLinkCXXStdlib(Args))
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
+  CmdArgs.push_back("-lm");
+}
 CmdArgs.push_back("--start-group");

[clang] f4dd977 - [AST] Use canonical constraint declaration for ASTContext::getAutoType

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-07-04T17:38:05+08:00
New Revision: f4dd977537dc0fcd8605a1ce066a4c7fd271a5d7

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

LOG: [AST] Use canonical constraint declaration for ASTContext::getAutoType

When we do profiling in ASTContext::getAutoType, it wouldn't think about
the canonical declaration for the type constraint. It is bad since it
would cause a negative ODR mismatch while we already know the type
constraint declaration is a redeclaration for the previous one. Also it 
shouldn't be
bad to use the canonical declaration here.

Added: 


Modified: 
clang/lib/AST/ASTContext.cpp
clang/test/Modules/concept.cppm

Removed: 




diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 682b71a3d6865..e64135cbf3f4e 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -5707,6 +5707,9 @@ QualType ASTContext::getAutoTypeInternal(
   !TypeConstraintConcept && !IsDependent)
 return getAutoDeductType();
 
+  if (TypeConstraintConcept)
+TypeConstraintConcept = TypeConstraintConcept->getCanonicalDecl();
+
   // Look in the folding set for an existing type.
   void *InsertPos = nullptr;
   llvm::FoldingSetNodeID ID;

diff  --git a/clang/test/Modules/concept.cppm b/clang/test/Modules/concept.cppm
index f171e6d082374..84bc0fef83926 100644
--- a/clang/test/Modules/concept.cppm
+++ b/clang/test/Modules/concept.cppm
@@ -1,10 +1,54 @@
 // RUN: rm -rf %t
 // RUN: mkdir %t
-// RUN: %clang_cc1 -x c++ -std=c++20 %S/Inputs/concept/A.cppm 
-emit-module-interface -o %t/A.pcm
-// RUN: %clang_cc1 -x c++ -std=c++20 -fprebuilt-module-path=%t 
-I%S/Inputs/concept %s -fsyntax-only -verify
-// expected-no-diagnostics
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-module-interface -o %t/A.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/B.cppm -verify
+
+//--- foo.h
+#ifndef FOO_H
+#define FOO_H
+
+template 
+concept Range = requires(T ) { t.begin(); };
+
+template
+concept __integer_like = true;
+
+template 
+concept __member_size = requires(_Tp &) { t.size(); };
+
+struct A {
+public:
+  template 
+  using range_type = T;
+};
 
+struct __fn {
+  template <__member_size _Tp>
+  constexpr __integer_like auto operator()(_Tp&& __t) const {
+return __t.size();
+  }
+};
+#endif
+
+//--- A.cppm
+module;
+#include "foo.h"
+export module A;
+
+//--- B.cppm
+// expected-no-diagnostics
 module;
 #include "foo.h"
 export module B;
 import A;
+
+void foo() {
+A a;
+struct S {
+int size() { return 0; }
+auto operator+(S s) { return 0; }
+};
+__fn{}(S());
+}



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

[PATCH] D129064: [clang-format] Avoid crash in LevelIndentTracker.

[Marek Kurdej via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129064

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTestSelective.cpp


Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -609,6 +609,14 @@
  "  return a == 8 ? 32 : 16;\n"
  "}\n";
   EXPECT_EQ(Code, format(Code, 40, 0));
+
+  // https://llvm.org/PR56352
+  Style.CompactNamespaces = true;
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  Code = "\n"
+ "namespace ns1 { namespace ns2 {\n"
+ "}}";
+  EXPECT_EQ(Code, format(Code, 0, 0));
 }
 
 } // end namespace
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -66,7 +66,8 @@
   (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
-  IndentForLevel.resize(Line.Level + 1);
+  if (IndentForLevel.size() < Line.Level + 1)
+IndentForLevel.resize(Line.Level + 1);
   Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
@@ -91,6 +92,7 @@
 unsigned LevelIndent = Line.First->OriginalColumn;
 if (static_cast(LevelIndent) - Offset >= 0)
   LevelIndent -= Offset;
+assert(Line.Level < IndentForLevel.size());
 if ((!Line.First->is(tok::comment) || IndentForLevel[Line.Level] == -1) &&
 !Line.InPPDirective) {
   IndentForLevel[Line.Level] = LevelIndent;


Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -609,6 +609,14 @@
  "  return a == 8 ? 32 : 16;\n"
  "}\n";
   EXPECT_EQ(Code, format(Code, 40, 0));
+
+  // https://llvm.org/PR56352
+  Style.CompactNamespaces = true;
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  Code = "\n"
+ "namespace ns1 { namespace ns2 {\n"
+ "}}";
+  EXPECT_EQ(Code, format(Code, 0, 0));
 }
 
 } // end namespace
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -66,7 +66,8 @@
   (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
-  IndentForLevel.resize(Line.Level + 1);
+  if (IndentForLevel.size() < Line.Level + 1)
+IndentForLevel.resize(Line.Level + 1);
   Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
@@ -91,6 +92,7 @@
 unsigned LevelIndent = Line.First->OriginalColumn;
 if (static_cast(LevelIndent) - Offset >= 0)
   LevelIndent -= Offset;
+assert(Line.Level < IndentForLevel.size());
 if ((!Line.First->is(tok::comment) || IndentForLevel[Line.Level] == -1) &&
 !Line.InPPDirective) {
   IndentForLevel[Line.Level] = LevelIndent;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 5f0a054 - [pseudo] Remove duplicated code in ClangPseudo.cpp

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-07-04T11:32:56+02:00
New Revision: 5f0a054f8954d83aea66bac0ffa27887ff2eaade

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

LOG: [pseudo] Remove duplicated code in ClangPseudo.cpp

The code was added accidently during the rebase when landing fe66aebd.

Added: 


Modified: 
clang-tools-extra/pseudo/tool/ClangPseudo.cpp

Removed: 




diff  --git a/clang-tools-extra/pseudo/tool/ClangPseudo.cpp 
b/clang-tools-extra/pseudo/tool/ClangPseudo.cpp
index 521af05d5babf..295b7dc1d0087 100644
--- a/clang-tools-extra/pseudo/tool/ClangPseudo.cpp
+++ b/clang-tools-extra/pseudo/tool/ClangPseudo.cpp
@@ -151,39 +151,21 @@ int main(int argc, char *argv[]) {
 if (PrintForest)
   llvm::outs() << Root.dumpRecursive(Lang.G, /*Abbreviated=*/true);
 
-if (ParseableStream) {
-  clang::pseudo::ForestArena Arena;
-  clang::pseudo::GSS GSS;
-  llvm::Optional StartSymID =
-  Lang.G.findNonterminal(StartSymbol);
-  if (!StartSymID) {
-llvm::errs() << llvm::formatv(
-"The start symbol {0} doesn't exit in the grammar!\n", 
StartSymbol);
-return 2;
-  }
-  auto  =
-  glrParse(*ParseableStream,
-   clang::pseudo::ParseParams{Lang.G, Lang.Table, Arena, GSS},
-   *StartSymID);
-  if (PrintForest)
-llvm::outs() << Root.dumpRecursive(Lang.G, /*Abbreviated=*/true);
-
-  if (PrintStatistics) {
-llvm::outs() << "Forest bytes: " << Arena.bytes()
- << " nodes: " << Arena.nodeCount() << "\n";
-llvm::outs() << "GSS bytes: " << GSS.bytes()
- << " nodes: " << GSS.nodesCreated() << "\n";
-
-for (auto  :
- {std::make_pair("Ambiguous", 
clang::pseudo::ForestNode::Ambiguous),
-  std::make_pair("Opaque", clang::pseudo::ForestNode::Opaque)}) {
-  clang::pseudo::NodeStats Stats(
-  Root, [&](const auto ) { return N.kind() == P.second; });
-  llvm::outs() << "\n" << Stats.Total << " " << P.first << " nodes:\n";
-  for (const auto  : Stats.BySymbol)
-llvm::outs() << llvm::formatv("  {0,3} {1}\n", S.second,
-  Lang.G.symbolName(S.first));
-}
+if (PrintStatistics) {
+  llvm::outs() << "Forest bytes: " << Arena.bytes()
+   << " nodes: " << Arena.nodeCount() << "\n";
+  llvm::outs() << "GSS bytes: " << GSS.bytes()
+   << " nodes: " << GSS.nodesCreated() << "\n";
+
+  for (auto  :
+   {std::make_pair("Ambiguous", clang::pseudo::ForestNode::Ambiguous),
+std::make_pair("Opaque", clang::pseudo::ForestNode::Opaque)}) {
+clang::pseudo::NodeStats Stats(
+Root, [&](const auto ) { return N.kind() == P.second; });
+llvm::outs() << "\n" << Stats.Total << " " << P.first << " nodes:\n";
+for (const auto  : Stats.BySymbol)
+  llvm::outs() << llvm::formatv("  {0,3} {1}\n", S.second,
+Lang.G.symbolName(S.first));
   }
 }
   }



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

[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D129045#3627878 , @iains wrote:

> In D129045#3627856 , @ChuanqiXu 
> wrote:
>
>> It looks like the tests lack the cast that the methods are attached to the 
>> global module fragment.
>
> (maybe I misunderstand what you are saying here)
>
>   bool ImplicitInlineCXX20 = !getLangOpts().CPlusPlus20 ||
>  !NewFD->getOwningModule() ||
>  NewFD->getOwningModule()->isGlobalModule();
>
> We are in the global module if there is no owning module, or if the owning 
> module is the GMF. 
>  We always set this to true before C++20, matching the unconditional true 
> default argument it the setImplicitlyInline() method.

I mean the tests instead of the implementation. It looks like there is no GMF 
in the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129045

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

[PATCH] D128981: [C++20][Modules] Implement include translation.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/include/clang/Lex/Preprocessor.h:434
+/// Saw a 'module' identifier.
+void handleModule(bool ATLTS) {
+  // This was the first module identifier and not preceded by any token

iains wrote:
> ChuanqiXu wrote:
> > What's the meaning of `ATLTS`?
> `AfterTopLevelTokenSeq` I guess it seemed a rather long name (but I do not 
> mind to spell it out if that makes sense).
> 
Oh, I feel better to spell it out.



Comment at: clang/lib/Lex/PPDirectives.cpp:2226-2227
+
+  // FIXME: We do not have a good way to disambiguate C++ clang modules from
+  // C++ standard modules (other than use/non-use of Header Units).
+  Module *SM = SuggestedModule.getModule();

iains wrote:
> ChuanqiXu wrote:
> > From what @rsmith said in https://reviews.llvm.org/D113391, it looks like 
> > the ideal direction is to use C++ clang modules and C++ standard modules 
> > together. So it looks like we couldn't disambiguate them from command line 
> > options.
> Well, I think there is some more debate to have around how to solve this 
> problem (i.e. it might be intended that clang++ modules and standard c++ 
> modules converge but as things stand we still need to support the cases that 
> they have different behaviour, or break existing users) 
>  ... - but please let us not have that debate in this patch :-)
> 
It is not good to break existing users. Generally, a breaking change patch 
could be reverted directly... We must care about it to avoid unnecessary 
efforts. And it looks like the current implementation wouldn't break any 
existing users, right? Since it uses `isHeaderUnit()`. I remember `HeaderUnit` 
is introduced by  you so it shouldn't conflict with clang modules.

BTW, may I ask the behavior is consistency with GCC?



Comment at: clang/lib/Lex/PPDirectives.cpp:2335
+ IsFirstIncludeOfFile)) {
+// standard modules:
+// If we are not in the GMF, then we textually include only

nit: It looks like we prefer to use `C++20 modules` over `standard modules`, 
although `standard modules` must be the right term.



Comment at: clang/lib/Parse/Parser.cpp:672
+if (!getLangOpts().CPlusPlusModules || !Mod->isHeaderUnit() ||
+getLangOpts().ModulesTS)
+  Actions.ActOnModuleInclude(Loc, Mod);

I think we could ignore `getLangOpts().ModulesTS` here.



Comment at: clang/test/Modules/cxx20-include-translation.cpp:10
+// RUN: %clang_cc1 -std=c++20 -xc++-user-header h4.h -emit-header-unit -o 
h4.pcm
+
+// RUN: %clang_cc1 -std=c++20 Xlate.cpp -emit-module-interface -o Xlate.pcm \

iains wrote:
> ChuanqiXu wrote:
> > Maybe we need an example `h5.h` which is not pre-compiled  as a header unit 
> > and it would be handled correctly.
> I am not sure if you mean that the un-converted header would be included in 
> the GMF or in the module purview - IMO we already have test-cases that cover 
> this, but I do not mind adding something extra if there's a missing case?
> 
> I am not sure if you mean that the un-converted header would be included in 
> the GMF or in the module purview 

At least in the GMF. 

> IMO we already have test-cases that cover this, but I do not mind adding 
> something extra if there's a missing case?

Yeah, I feel better to have more coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128981

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

[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D129045#3627856 , @ChuanqiXu wrote:

> It looks like the tests lack the cast that the methods are attached to the 
> global module fragment.

(maybe I misunderstand what you are saying here)

  bool ImplicitInlineCXX20 = !getLangOpts().CPlusPlus20 ||
 !NewFD->getOwningModule() ||
 NewFD->getOwningModule()->isGlobalModule();

We are in the global module if there is no owning module, or if the owning 
module is the GMF. 
 We always set this to true before C++20, matching the unconditional true 
default argument it the setImplicitlyInline() method.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129045

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

[PATCH] D128645: Update developer policy.

[Edd Barrett via Phabricator via cfe-commits]

vext01 closed this revision.
vext01 added a comment.

Just pushed this to main (04f6bf482b8641533274d28af5fdac7107da3344 
)

Thanks everyone!


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

https://reviews.llvm.org/D128645

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

[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

It looks like the tests lack the cast that the methods are attached to the 
global module fragment.




Comment at: clang/lib/Sema/SemaDecl.cpp:9363-9365
+Module *M = NewFD->getOwningModule();
+if (!M || M->isGlobalModule())
+  NewFD->setImplicitlyInline();

iains wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > nit: this is not  required.
> > well, the logic is required ;) .. you have suggested a different way to 
> > write..
> actually, I think you meant `!NewFD->getOwningModule() || 
> NewFD->getOwningModule()->isGlobalModule()` but I've pulled the test out so 
> that it's only done once.
> 
Yeah, thanks!



Comment at: clang/test/AST/ast-dump-constant-expr.cpp:94
+// CHECK-NEXT:`-CXXMethodDecl {{.*}}  col:18 consteval 
consteval_method 'void ()' implicit-inline
\ No newline at end of file


Maybe it is good to add the newline although it is not your fault.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129045

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

[PATCH] D111081: [clang] [MinGW] Fix paths on Gentoo

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Does Gentoo patch upstream to do something different? (`.../g++-v10/...`) Well, 
I wish that Gentoo does not do this.
This adds complexity to Clang which in practice tries to provide some drop-in 
replacement ability.


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

https://reviews.llvm.org/D111081

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

[PATCH] D128981: [C++20][Modules] Implement include translation.

[Iain Sandoe via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Lex/Preprocessor.h:434
+/// Saw a 'module' identifier.
+void handleModule(bool ATLTS) {
+  // This was the first module identifier and not preceded by any token

ChuanqiXu wrote:
> What's the meaning of `ATLTS`?
`AfterTopLevelTokenSeq` I guess it seemed a rather long name (but I do not mind 
to spell it out if that makes sense).




Comment at: clang/lib/Lex/PPDirectives.cpp:2226-2227
+
+  // FIXME: We do not have a good way to disambiguate C++ clang modules from
+  // C++ standard modules (other than use/non-use of Header Units).
+  Module *SM = SuggestedModule.getModule();

ChuanqiXu wrote:
> From what @rsmith said in https://reviews.llvm.org/D113391, it looks like the 
> ideal direction is to use C++ clang modules and C++ standard modules 
> together. So it looks like we couldn't disambiguate them from command line 
> options.
Well, I think there is some more debate to have around how to solve this 
problem (i.e. it might be intended that clang++ modules and standard c++ 
modules converge but as things stand we still need to support the cases that 
they have different behaviour, or break existing users) 
 ... - but please let us not have that debate in this patch :-)




Comment at: clang/test/Modules/cxx20-include-translation.cpp:10
+// RUN: %clang_cc1 -std=c++20 -xc++-user-header h4.h -emit-header-unit -o 
h4.pcm
+
+// RUN: %clang_cc1 -std=c++20 Xlate.cpp -emit-module-interface -o Xlate.pcm \

ChuanqiXu wrote:
> Maybe we need an example `h5.h` which is not pre-compiled  as a header unit 
> and it would be handled correctly.
I am not sure if you mean that the un-converted header would be included in the 
GMF or in the module purview - IMO we already have test-cases that cover this, 
but I do not mind adding something extra if there's a missing case?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128981

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

[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

[Iain Sandoe via Phabricator via cfe-commits]

iains added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:9363-9365
+Module *M = NewFD->getOwningModule();
+if (!M || M->isGlobalModule())
+  NewFD->setImplicitlyInline();

iains wrote:
> ChuanqiXu wrote:
> > nit: this is not  required.
> well, the logic is required ;) .. you have suggested a different way to 
> write..
actually, I think you meant `!NewFD->getOwningModule() || 
NewFD->getOwningModule()->isGlobalModule()` but I've pulled the test out so 
that it's only done once.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129045

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

[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 442029.
iains marked 4 inline comments as done.
iains added a comment.

rebased, factored code to address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129045

Files:
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/AST/ast-dump-constant-expr.cpp
  clang/test/AST/ast-dump-lambda.cpp
  clang/test/CXX/class/class.friend/p7-cxx20.cpp
  clang/test/CXX/class/class.mfct/p1-cxx20.cpp

Index: clang/test/CXX/class/class.mfct/p1-cxx20.cpp
===
--- /dev/null
+++ clang/test/CXX/class/class.mfct/p1-cxx20.cpp
@@ -0,0 +1,42 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 no-modules.cpp -fsyntax-only -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-NM %s
+// RUN: %clang_cc1 -std=c++20 -xc++-user-header header-unit.h -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-HU %s
+// RUN: %clang_cc1 -std=c++20 module.cpp -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-MOD %s
+
+//--- no-modules.cpp
+
+class X {
+  void x(){};
+};
+
+// CHECK-NM: `-CXXRecordDecl {{.*}}  line:2:7 class X definition
+// CHECK-NM:   |-CXXRecordDecl {{.*}}  col:7 implicit class X
+// CHECK-NM-NEXT: `-CXXMethodDecl {{.*}}  col:8 x 'void ()' implicit-inline
+
+//--- header-unit.h
+
+class Y {
+  void y(){};
+};
+
+// CHECK-HU: `-CXXRecordDecl {{.*}} <./header-unit.h:2:1, line:4:1> line:2:7 class Y definition
+// CHECK-HU: |-CXXRecordDecl {{.*}}  col:7 implicit class Y
+// CHECK-HU-NEXT: `-CXXMethodDecl {{.*}}  col:8 y 'void ()' implicit-inline
+
+//--- module.cpp
+export module M;
+
+class Z {
+  void z(){};
+};
+
+// CHECK-MOD: `-CXXRecordDecl {{.*}}  line:3:7 in M hidden class Z{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: |-CXXRecordDecl {{.*}}  col:7 in M hidden implicit class Z{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: `-CXXMethodDecl {{.*}}  col:8 in M hidden z 'void ()'{{( ReachableWhenImported)?}}
Index: clang/test/CXX/class/class.friend/p7-cxx20.cpp
===
--- /dev/null
+++ clang/test/CXX/class/class.friend/p7-cxx20.cpp
@@ -0,0 +1,45 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 no-modules.cpp -fsyntax-only -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-NM %s
+// RUN: %clang_cc1 -std=c++20 -xc++-user-header header-unit.h -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-HU %s
+// RUN: %clang_cc1 -std=c++20 module.cpp -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-MOD %s
+
+//--- no-modules.cpp
+
+class X {
+  friend void x(){};
+};
+
+// CHECK-NM: `-CXXRecordDecl {{.*}}  line:2:7 class X definition
+// CHECK-NM:   |-CXXRecordDecl {{.*}}  col:7 implicit class X
+// CHECK-NM-NEXT: `-FriendDecl {{.*}}  col:15
+// CHECK-NM-NEXT: `-FunctionDecl {{.*}} parent {{.*}}  col:15 x 'void ()' implicit-inline
+
+//--- header-unit.h
+
+class Y {
+  friend void y(){};
+};
+
+// CHECK-HU: `-CXXRecordDecl {{.*}} <./header-unit.h:2:1, line:4:1> line:2:7 class Y definition
+// CHECK-HU: |-CXXRecordDecl {{.*}}  col:7 implicit class Y
+// CHECK-HU-NEXT: `-FriendDecl {{.*}}  col:15
+// CHECK-HU-NEXT: `-FunctionDecl {{.*}} parent {{.*}}  col:15 y 'void ()' implicit-inline
+
+//--- module.cpp
+export module M;
+
+class Z {
+  friend void z(){};
+};
+
+// CHECK-MOD: `-CXXRecordDecl {{.*}}  line:3:7 in M hidden class Z{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: |-CXXRecordDecl {{.*}}  col:7 in M hidden implicit class Z{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: `-FriendDecl {{.*}}  col:15 in M{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: `-FunctionDecl {{.*}} parent {{.*}}  col:15 in M hidden z 'void ()'{{( ReachableWhenImported)?}}
Index: clang/test/AST/ast-dump-lambda.cpp
===
--- clang/test/AST/ast-dump-lambda.cpp
+++ clang/test/AST/ast-dump-lambda.cpp
@@ -51,7 +51,7 @@
 // CHECK-NEXT:|   | |-MoveAssignment exists simple trivial needs_implicit
 // CHECK-NEXT:|   | `-Destructor simple irrelevant trivial needs_implicit
 // CHECK-NEXT:|   |-CXXRecordDecl {{.*}}  col:10{{( imported)?}} implicit struct V
-// CHECK-NEXT:|   `-CXXMethodDecl {{.*}}  line:17:10{{( imported)?}} f 'void ()'
+// CHECK-NEXT:|   `-CXXMethodDecl {{.*}}  line:17:10{{( imported)?}} f 'void ()' implicit-inline
 // CHECK-NEXT:| `-CompoundStmt {{.*}} 
 // CHECK-NEXT:|   |-LambdaExpr {{.*}}  '(lambda at {{.*}}ast-dump-lambda.cpp:18:7)'
 // CHECK-NEXT:|   | |-CXXRecordDecl {{.*}}  col:7{{( imported)?}} implicit{{( )?}} class definition
Index: clang/test/AST/ast-dump-constant-expr.cpp

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added inline comments.



Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:100
+switch (Type->getElementBitwidth()) {
+case 64:
+  QT = Context.DoubleTy;

aaron.ballman wrote:
> kito-cheng wrote:
> > aaron.ballman wrote:
> > > I almost hate to ask, but... `long double`? Any of the 16-bit float types?
> > Have 16 bit floating below, but we don't support long double in our 
> > intrinsic for now, add an assertion to make sure.
> Very glad to hear about `long double`, but I was unclear on the 16-bit float, 
> I was more wondering if you need to differentiate between `Float16Ty`, 
> `BFloat16Ty`, and `HalfTy` since those will all have the same bit widths but 
> be different types.
RISC-V vector intrinsic only support `_Float16`(`Float16Ty`) for now, 
`__fp16`(`HalfTy`, `half` in OpenCL) won't support, `BFloat16Ty` will be 
another `ScalarTypeKind` like `ScalarTypeKind::BFloat`, we didn't add yet since 
we don't have any `bfloat16` instruction  in RISC-V extension now.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:637
+Out.emplace_back(Record);
+Out.back().Name = SR.Name.c_str();
+Out.back().OverloadedName = SR.OverloadedName.c_str();

frasercrmck wrote:
> I assume the compiler's able to avoid recomputing `Out.back()` multiple 
> times? We could take a reference to `Out.back()` and use that, just in case?
Checked code gen with following program, got almost same code gen:

```
#include 
struct X{
int a;
int b;
int c;
};

#ifdef BACK
void foo(std::vector ){
X x;
Out.emplace_back(x);
Out.back().a =12;
Out.back().b =23;
Out.back().c =30;
}
#else
void foo2(std::vector ){
Out.emplace_back(X());
X  = Out.back();
x.a =12;
x.b =23;
x.c =30;
}
#endif
```

But I think later one might be more readable, let me tweak that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617

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

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng updated this revision to Diff 442025.
kito-cheng marked 9 inline comments as done.
kito-cheng added a comment.

Changes:

- Address @frasercrmck's comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617

Files:
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/RISCVIntrinsicManager.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Support/RISCVVIntrinsicUtils.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaRISCVVectorLookup.cpp
  clang/lib/Support/RISCVVIntrinsicUtils.cpp
  clang/test/Sema/riscv-bad-intrnisic-pragma.c
  clang/test/Sema/riscv-intrnisic-pragma.c
  clang/utils/TableGen/RISCVVEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -110,6 +110,7 @@
 void EmitRVVHeader(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitRVVBuiltins(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitRVVBuiltinCG(llvm::RecordKeeper , llvm::raw_ostream );
+void EmitRVVBuiltinSema(llvm::RecordKeeper , llvm::raw_ostream );
 
 void EmitCdeHeader(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitCdeBuiltinDef(llvm::RecordKeeper , llvm::raw_ostream );
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -88,6 +88,7 @@
   GenRISCVVectorHeader,
   GenRISCVVectorBuiltins,
   GenRISCVVectorBuiltinCG,
+  GenRISCVVectorBuiltinSema,
   GenAttrDocs,
   GenDiagDocs,
   GenOptDocs,
@@ -243,6 +244,8 @@
"Generate riscv_vector_builtins.inc for clang"),
 clEnumValN(GenRISCVVectorBuiltinCG, "gen-riscv-vector-builtin-codegen",
"Generate riscv_vector_builtin_cg.inc for clang"),
+clEnumValN(GenRISCVVectorBuiltinSema, "gen-riscv-vector-builtin-sema",
+   "Generate riscv_vector_builtin_sema.inc for clang"),
 clEnumValN(GenAttrDocs, "gen-attr-docs",
"Generate attribute documentation"),
 clEnumValN(GenDiagDocs, "gen-diag-docs",
@@ -458,6 +461,9 @@
   case GenRISCVVectorBuiltinCG:
 EmitRVVBuiltinCG(Records, OS);
 break;
+  case GenRISCVVectorBuiltinSema:
+EmitRVVBuiltinSema(Records, OS);
+break;
   case GenAttrDocs:
 EmitClangAttrDocs(Records, OS);
 break;
Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
@@ -29,6 +30,59 @@
 using namespace clang::RISCV;
 
 namespace {
+struct SemaRecord {
+  // Intrinsic name, e.g. vadd_vv
+  std::string Name;
+
+  // Overloaded intrinsic name, could be empty if can be computed from Name
+  // e.g. vadd
+  std::string OverloadedName;
+
+  // Supported type, mask of BasicType.
+  unsigned TypeRangeMask;
+
+  // Supported LMUL.
+  unsigned Log2LMULMask;
+
+  // Required extensions for this intrinsic.
+  unsigned RequiredExtensions;
+
+  // Prototype for this intrinsic.
+  SmallVector Prototype;
+
+  // Prototype for masked intrinsic.
+  SmallVector MaskedPrototype;
+
+  // Suffix of intrinsic name.
+  SmallVector Suffix;
+
+  // Suffix of overloaded intrinsic name.
+  SmallVector OverloadedSuffix;
+
+  // Number of field, large than 1 if it's segment load/store.
+  unsigned NF;
+};
+
+// Compressed function signature table.
+class SemaSignatureTable {
+private:
+  std::vector SignatureTable;
+
+  void insert(ArrayRef Signature);
+
+public:
+  static constexpr unsigned INVALID_INDEX = ~0U;
+
+  // Create compressed signature table from SemaRecords.
+  void init(ArrayRef SemaRecords);
+
+  // Query the Signature, return INVALID_INDEX if not found.
+  unsigned getIndex(ArrayRef Signature);
+
+  /// Print signature table in RVVHeader Record to \p OS
+  void print(raw_ostream );
+};
+
 class RVVEmitter {
 private:
   RecordKeeper 
@@ -45,22 +99,22 @@
   /// Emit all the information needed to map builtin -> LLVM IR intrinsic.
   void createCodeGen(raw_ostream );
 
+  /// Emit all the information needed by SemaRISCVVectorLookup.cpp.
+  /// We've large number of intrinsic function for RVV, creating a customized
+  /// could speed up the compilation time.
+  void createSema(raw_ostream );
+
 private:
-  /// Create 

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.
Herald added a reviewer: NoQ.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-const-correctness.rst:10
+`CppCoreGuidelines ES.25 
`_
+and `AUTOSAR C++14 Rule A7-1-1 (6.7.1 Specifiers) 
`_.
+

AUTOSAR mentions should be removed as per previous comment from @aaron.ballman 


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

https://reviews.llvm.org/D54943

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

[PATCH] D118996: [clang-tidy] Support C++14 in bugprone-signal-handler.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 442024.
balazske added a comment.

Sorted the whole changed-check list in release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118996

Files:
  clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/signal-handler.rst
  clang-tools-extra/docs/clang-tidy/checks/cert/msc54-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/signal-handler/stdcpp.h
  clang-tools-extra/test/clang-tidy/checkers/bugprone/signal-handler.c
  clang-tools-extra/test/clang-tidy/checkers/bugprone/signal-handler.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/signal-handler.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/signal-handler.cpp
@@ -0,0 +1,228 @@
+// RUN: %check_clang_tidy -std=c++14 %s bugprone-signal-handler %t -- -- -isystem %clang_tidy_headers -isystem %S/Inputs/signal-handler
+
+#include "stdcpp.h"
+#include "stdio.h"
+
+// Functions called "signal" that are different from the system version.
+typedef void (*callback_t)(int);
+void signal(int, callback_t, int);
+namespace ns {
+void signal(int, callback_t);
+}
+
+extern "C" void handler_unsafe(int) {
+  printf("xxx");
+}
+
+extern "C" void handler_unsafe_1(int) {
+  printf("xxx");
+}
+
+namespace test_invalid_handler {
+
+void handler_non_extern_c(int) {
+  printf("xxx");
+}
+
+struct A {
+  static void handler_member(int) {
+printf("xxx");
+  }
+};
+
+void test() {
+  std::signal(SIGINT, handler_unsafe_1);
+  // CHECK-MESSAGES: :[[@LINE-17]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:23: note: function 'handler_unsafe_1' registered here as signal handler
+
+  std::signal(SIGINT, handler_non_extern_c);
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: functions without C linkage are not allowed as signal handler (until C++17) [bugprone-signal-handler]
+  std::signal(SIGINT, A::handler_member);
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: functions without C linkage are not allowed as signal handler (until C++17) [bugprone-signal-handler]
+  std::signal(SIGINT, [](int) { printf("xxx"); });
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: lambda function is not allowed as signal handler (until C++17) [bugprone-signal-handler]
+
+  // This case is (deliberately) not found by the checker.
+  std::signal(SIGINT, [](int) -> callback_t { return _unsafe; }(1));
+}
+
+} // namespace test_invalid_handler
+
+namespace test_non_standard_signal_call {
+
+struct Signal {
+  static void signal(int, callback_t);
+};
+
+void test() {
+  // No diagnostics here. All these signal calls differ from the standard system one.
+  signal(SIGINT, handler_unsafe, 1);
+  ns::signal(SIGINT, handler_unsafe);
+  Signal::signal(SIGINT, handler_unsafe);
+  system_other::signal(SIGINT, handler_unsafe);
+}
+
+} // namespace test_non_standard_signal_call
+
+namespace test_cpp_construct_in_handler {
+
+struct Struct {
+  virtual ~Struct() {}
+  void f1();
+  int *begin();
+  int *end();
+  static void f2();
+};
+struct Derived : public Struct {
+};
+
+struct X {
+  X(int, float);
+};
+
+Struct *S_Global;
+const Struct *S_GlobalConst;
+
+void f_non_extern_c() {
+}
+
+void f_default_arg(int P1 = 0) {
+}
+
+extern "C" void handler_cpp(int) {
+  using namespace ::test_cpp_construct_in_handler;
+
+  // These calls are not found as problems.
+  // (Called functions are not analyzed if the current function has already
+  // other problems.)
+  f_non_extern_c();
+  Struct::f2();
+  // 'auto' is not disallowed
+  auto Auto = 28u;
+
+  Struct S;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: remark: internally, the statement is parsed as a 'CXXConstructExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  S_Global->f1();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXMemberCallExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  const Struct  = Struct();
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: C++-only construct is not allowed in signal handler (until C++17) 

[PATCH] D129057: [clang-format] Break on AfterColon only if not followed by comment

[Marek Kurdej via Phabricator via cfe-commits]

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

LGTM. Thanks for addressing my comments.


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

https://reviews.llvm.org/D129057

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

[PATCH] D129061: [Lex] Diagnose macro in command lines

[Jack Huang via Phabricator via cfe-commits]

jackhong12 created this revision.
jackhong12 added a reviewer: aaron.ballman.
Herald added a project: All.
jackhong12 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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

Description
---

We need to diagnose the macro in commands too. For example, the expected result 
of `clang++ -Wreserved-identifier -D__WHY_NOT_ME__ -U__STDC__ tmp.cpp` should be

  :1:9: warning: macro name is a reserved identifier 
[-Wreserved-macro-identifier]
  #define __WHY_NOT_ME__ 1
  ^
  :2:8: warning: macro name is a reserved identifier 
[-Wreserved-macro-identifier]
  #undef __STDC__
 ^

Currently, clang will not show any warning message when we define or undefine 
reserved macro in commands.

Fix
---

We use digital directives in the preprocessor, like

  # 1 "" 3
  #define __llvm__ 1
  #define __clang__ 1
  
  ...
  
  # 1 "" 1
  #define __WHY_NOT_ME__ 1
  #undef __STDC__
  #define __GCC_HAVE_DWARF2_CFI_ASM 1
  # 1 "" 2

I think we should use `PresumedLoc` to determine its `` or `` section. In this case, the file name will be changed from `` 
to ``.  However,  `SourceMgr.getBufferName` only returns the 
first name it matches.

Another issue is that clang will define `__GCC_HAVE_DWARF2_CFI_ASM` in the 
command line. But I don't know how to handle this macro. I only add this macro 
to the reserved macro list.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129061

Files:
  clang/lib/Lex/PPDirectives.cpp


Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -164,6 +164,7 @@
 "_THREAD_SAFE",
 "_XOPEN_SOURCE",
 "_XOPEN_SOURCE_EXTENDED",
+"__GCC_HAVE_DWARF2_CFI_ASM",
 "__STDCPP_WANT_MATH_SPEC_FUNCS__",
 "__STDC_FORMAT_MACROS",
 };
@@ -352,8 +353,9 @@
   SourceLocation MacroNameLoc = MacroNameTok.getLocation();
   if (ShadowFlag)
 *ShadowFlag = false;
-  if (!SourceMgr.isInSystemHeader(MacroNameLoc) &&
-  (SourceMgr.getBufferName(MacroNameLoc) != "")) {
+  PresumedLoc PL = SourceMgr.getPresumedLoc(MacroNameLoc);
+  if (!SourceMgr.isInSystemHeader(MacroNameLoc) && PL.isValid() &&
+  strcmp(PL.getFilename(), "")) {
 MacroDiag D = MD_NoWarn;
 if (isDefineUndef == MU_Define) {
   D = shouldWarnOnMacroDef(*this, II);


Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -164,6 +164,7 @@
 "_THREAD_SAFE",
 "_XOPEN_SOURCE",
 "_XOPEN_SOURCE_EXTENDED",
+"__GCC_HAVE_DWARF2_CFI_ASM",
 "__STDCPP_WANT_MATH_SPEC_FUNCS__",
 "__STDC_FORMAT_MACROS",
 };
@@ -352,8 +353,9 @@
   SourceLocation MacroNameLoc = MacroNameTok.getLocation();
   if (ShadowFlag)
 *ShadowFlag = false;
-  if (!SourceMgr.isInSystemHeader(MacroNameLoc) &&
-  (SourceMgr.getBufferName(MacroNameLoc) != "")) {
+  PresumedLoc PL = SourceMgr.getPresumedLoc(MacroNameLoc);
+  if (!SourceMgr.isInSystemHeader(MacroNameLoc) && PL.isValid() &&
+  strcmp(PL.getFilename(), "")) {
 MacroDiag D = MD_NoWarn;
 if (isDefineUndef == MU_Define) {
   D = shouldWarnOnMacroDef(*this, II);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129057: [clang-format] Break on AfterColon only if not followed by comment

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 442016.
owenpan added a comment.

Handle `PackContructorInitializers: NextLine` and address review comments.


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

https://reviews.llvm.org/D129057

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7034,6 +7034,10 @@
   "SomeClass::Constructor() :\n"
   "a(aa), aaa() {}",
   Style);
+  verifyFormat(
+  "SomeClass::Constructor() : // NOLINT\n"
+  "a(aa), aaa() {}",
+  Style);
 
   Style.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
   verifyFormat(
@@ -7101,6 +7105,10 @@
"a(aa),\n"
"a(aa) {}",
OnePerLine);
+  verifyFormat("Foo::Foo(int i, int j) : // NOLINT\n"
+   "i(i),// comment\n"
+   "j(j) {}",
+   OnePerLine);
   verifyFormat("MyClass::MyClass(int var) :\n"
"some_var_(var),// 4 space indent\n"
"some_other_var_(var + 1) { // lined up\n"
@@ -7133,6 +7141,17 @@
"// Comment forcing unwanted break.\n"
"() {}",
Style);
+  verifyFormat("Constructor() : // NOLINT\n"
+   "() {}",
+   Style);
+  verifyFormat("Constructor() : // A very long trailing comment that cannot 
fit"
+   " on a single\n"
+   "// line.\n"
+   "() {}",
+   "Constructor() : // A very long trailing comment that cannot 
fit"
+   " on a single line.\n"
+   "() {}",
+   Style);
 
   Style.ColumnLimit = 0;
   verifyFormat("SomeClass::Constructor() :\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4734,7 +4734,7 @@
 // the first list element. Otherwise, it should be placed outside of the
 // list.
 return Left.is(BK_BracedInit) ||
-   (Left.is(TT_CtorInitializerColon) &&
+   (Left.is(TT_CtorInitializerColon) && Right.NewlinesBefore > 0 &&
 Style.BreakConstructorInitializers == 
FormatStyle::BCIS_AfterColon);
   }
   if (Left.is(tok::question) && Right.is(tok::colon))
@@ -4894,8 +4894,10 @@
   if (Right.is(tok::identifier) && Right.Next && 
Right.Next->is(TT_DictLiteral))
 return true;
 
-  if (Left.is(TT_CtorInitializerColon))
-return Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon;
+  if (Left.is(TT_CtorInitializerColon)) {
+return Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon 
&&
+   (!Right.isTrailingComment() || Right.NewlinesBefore > 0);
+  }
   if (Right.is(TT_CtorInitializerColon))
 return Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon;
   if (Left.is(TT_CtorInitializerComma) &&
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -404,6 +404,7 @@
   (State.Column + State.Line->Last->TotalLength - Previous.TotalLength >
getColumnLimit(State) ||
CurrentState.BreakBeforeParameter) &&
+  (!Current.isTrailingComment() || Current.NewlinesBefore > 0) &&
   (Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All ||
Style.BreakConstructorInitializers != FormatStyle::BCIS_BeforeColon ||
Style.ColumnLimit != 0)) {
@@ -793,6 +794,7 @@
   (Previous.is(tok::colon) && Previous.is(TT_ObjCMethodExpr {
 CurrentState.LastSpace = State.Column;
   } else if (Previous.is(TT_CtorInitializerColon) &&
+ (!Current.isTrailingComment() || Current.NewlinesBefore > 0) &&
  Style.BreakConstructorInitializers ==
  FormatStyle::BCIS_AfterColon) {
 CurrentState.Indent = State.Column;
@@ -1032,7 +1034,7 @@
 // be considered bin packing unless the relevant AllowAll option is false 
or
 // this is a dict/object literal.
 bool PreviousIsBreakingCtorInitializerColon =
-Previous.is(TT_CtorInitializerColon) &&
+PreviousNonComment && PreviousNonComment->is(TT_CtorInitializerColon) 
&&
 Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon;
 if (!(Previous.isOneOf(tok::l_paren, tok::l_brace, TT_BinaryOperator) ||
   

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

@LegalizeAdulthood Thanks for the review! I've now rebased and addressed your 
comments. I've also verify the docs with the command you suggested, I was 
missing `-DLLVM_ENABLE_SPHINX` in the cmake command.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126880

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

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 442015.
carlosgalvezp marked 2 inline comments as done.
carlosgalvezp added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126880

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
@@ -0,0 +1,152 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-const-or-ref-data-members %t
+namespace std {
+template 
+struct reference_wrapper {};
+} // namespace std
+
+namespace gsl {
+template 
+struct not_null {};
+} // namespace gsl
+
+struct Ok {
+  int i;
+  int *p;
+  const int *pc;
+  std::reference_wrapper r;
+  gsl::not_null n;
+};
+
+struct ConstMember {
+  const int c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members]
+};
+
+struct LvalueRefMember {
+  int 
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+};
+
+struct ConstRefMember {
+  const int 
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+};
+
+struct RvalueRefMember {
+  int &
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+struct ConstAndRefMembers {
+  int const c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+  int 
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+  const int 
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+  int &
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+struct Foo {};
+
+struct Ok2 {
+  Foo i;
+  Foo *p;
+  const Foo *pc;
+  std::reference_wrapper r;
+  gsl::not_null n;
+};
+
+struct ConstMember2 {
+  const Foo c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+};
+
+struct LvalueRefMember2 {
+  Foo 
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+};
+
+struct ConstRefMember2 {
+  const Foo 
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+};
+
+struct RvalueRefMember2 {
+  Foo &
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+struct ConstAndRefMembers2 {
+  const Foo c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+  Foo 
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+  const Foo 
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+  Foo &
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+using ConstType = const int;
+using RefType = int &;
+using ConstRefType = const int &;
+using RefRefType = int &&;
+
+struct WithAlias {
+  ConstType c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+  RefType lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: member 'lr' is a reference
+  ConstRefType cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: member 'cr' is a reference
+  RefRefType rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'rr' is a reference
+};
+
+template 
+using Array = int[N];
+
+struct ConstArrayMember {
+  const Array<1> c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: member 'c' is const qualified
+};
+
+struct LvalueRefArrayMember {
+  Array<2> 
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'lr' is a reference
+};
+
+struct ConstLvalueRefArrayMember {
+  Array<3> const 
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: member 'cr' is a reference
+};
+
+struct RvalueRefArrayMember {
+  Array<4> &
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'rr' is a reference
+};
+
+template 
+struct TemplatedOk {
+  T t;
+};
+
+template 
+struct TemplatedConst {
+  T t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: member 't' is const qualified
+};
+
+template 
+struct TemplatedRef {
+  T t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: member 't' is a reference
+};
+
+TemplatedOk t1{};
+TemplatedConst t2{123};
+TemplatedRef t3{123};
+TemplatedRef t4{123};
+TemplatedRef t5{t1.t};
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst

[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

[Iain Sandoe via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/TextNodeDumper.cpp:1723-1725
+  if (!D->isInlineSpecified() && D->isInlined()) {
+OS << " implicit-inline";
+  }

ChuanqiXu wrote:
> Is this necessary to this revision?
yes, to test it



Comment at: clang/lib/Sema/SemaDecl.cpp:9362
+  //   to the global module.
+  if (getLangOpts().CPlusPlus20 || getLangOpts().CPlusPlus2b) {
+Module *M = NewFD->getOwningModule();

ChuanqiXu wrote:
> If I remember correctly, when we run `-std=c++2b`, 
> `getLangOpts().CPlusPlus20` would be true too. So we could check 
> `getLangOpts().CPlusPlus20` only here.
will double-check.



Comment at: clang/lib/Sema/SemaDecl.cpp:9363-9365
+Module *M = NewFD->getOwningModule();
+if (!M || M->isGlobalModule())
+  NewFD->setImplicitlyInline();

ChuanqiXu wrote:
> nit: this is not  required.
well, the logic is required ;) .. you have suggested a different way to write..



Comment at: clang/lib/Sema/SemaDecl.cpp:9657
+  //   in its class definition, it is inline.
+  if (getLangOpts().CPlusPlus20 || getLangOpts().CPlusPlus2b) {
+Module *M = NewFD->getOwningModule();

ChuanqiXu wrote:
> ditto
likewise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129045

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