[PATCH] D145138: [clang-tidy] Implement FixIts for C arrays

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp:588
+: ClangTidyCheck(Name, Context),
+  IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+   utils::IncludeSorter::IS_LLVM),

PiotrZSL wrote:
> ccotter wrote:
> > Eugene.Zelenko wrote:
> > > ccotter wrote:
> > > > Eugene.Zelenko wrote:
> > > > > Should be global option, because it's used in other checks.
> > > > Could you clarify this a bit? This is how most other tests consume 
> > > > IncludeStyle (`Options.getLocalOrGlobal("IncludeStyle", 
> > > > utils::IncludeSorter::IS_LLVM)`.
> > > @carlosgalvezp is best person to answer because he recently introduced 
> > > global option for source files and headers.
> > bump @carlosgalvezp 
> ```
> Local:
>   - key: modernize-avoid-c-arrays.IncludeStyle
>  value: google
> 
> Global:
>   - key: IncludeStyle
>  value: google
> ```
> 
> Your code is correct, simply if there is local defined, it will be used, if 
> there is global defined, then global will be used.
> 
Yep, this is similar to `HeaderFileExtensions`, current state is that all 
checks duplicate the same code when it should instead be just a global option. 
Since the global option is not available yet, let's leave it as it is and I 
will put up a patch to deprecate the local options.

Luckily this time it will be much easier since the value is just a string!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145138

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

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/lib/Parse/ParseStmt.cpp:554
+// Otherwise, eat the semicolon.
+ExpectAndConsumeSemi(diag::err_expected_semi_after_expr);
+  }

We seem to have more than one call to 
`ExpectAndConsumeSemi(diag::err_expected_semi_after_expr)`. Do we need 
something with them as well? Perhaps it is easier to have a check in 
`ExpectAndConsumeSemi` when the `diagID` is `err_expected_semi_after_expr` and 
check if `PP.isIncrementalProcessingEnabled() && Tok.is(tok::annot_input_end)`. 
There we can our new interface, say, `Sema::ActOnEndOfInput` which can attach 
more infrastructure if necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

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

[PATCH] D147377: [clang-format] Don't allow variable decls to have trailing return arrows

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1922
 } else if (Current.is(tok::arrow) && AutoFound &&
(Line.MustBeDeclaration || Line.InPPDirective) &&
Current.NestingLevel == 0 &&

It seems we can simply check if the line might be a function declaration here 
instead of resetting `AutoFound` below.



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:1493
+
+  Tokens = annotate("auto foo() -> auto { return Val; }");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;

Can this be valid C++?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147377

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

[PATCH] D147377: [clang-format] Don't allow variable decls to have trailing return arrows

[Emilia Dreamer via Phabricator via cfe-commits]

rymiel added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1922
 } else if (Current.is(tok::arrow) && AutoFound &&
(Line.MustBeDeclaration || Line.InPPDirective) &&
Current.NestingLevel == 0 &&

owenpan wrote:
> It seems we can simply check if the line might be a function declaration here 
> instead of resetting `AutoFound` below.
I could swear I specifically tried using that flag and it didn't work in all 
cases, but I guess I did something wrong back then...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147377

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

[PATCH] D147377: [clang-format] Don't allow variable decls to have trailing return arrows

[Emilia Dreamer via Phabricator via cfe-commits]

rymiel updated this revision to Diff 510315.
rymiel marked an inline comment as done.
rymiel added a comment.

Use MightBeFunctionDecl instead


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147377

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1468,6 +1468,72 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
+  auto Tokens = annotate("auto f() -> int;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator->() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator++(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto foo() -> auto { return Val; }");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("struct S { auto bar() const -> int; };");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::arrow, TT_TrailingReturnArrow);
+
+  // Not trailing return arrows
+  Tokens = annotate("auto a = b->c;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = (b)->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b->c();");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("decltype(auto) a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b->c(); }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[11], tok::arrow, TT_Unknown);
+
+  // Mixed
+  Tokens = annotate("auto f() -> int { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+  EXPECT_TOKEN(Tokens[13], tok::arrow, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
 return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1919,7 +1919,7 @@
Style.Language == FormatStyle::LK_Java) {
   Current.setType(TT_LambdaArrow);
 } else if (Current.is(tok::arrow) && AutoFound &&
-   (Line.MustBeDeclaration || Line.InPPDirective) &&
+   (Line.MightBeFunctionDecl || Line.InPPDirective) &&
Current.NestingLevel == 0 &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1468,6 +1468,72 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
+  auto Tokens = annotate("auto f() -> int;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator->() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator++(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Traili

[PATCH] D147377: [clang-format] Don't allow variable decls to have trailing return arrows

[Emilia Dreamer via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:1493
+
+  Tokens = annotate("auto foo() -> auto { return Val; }");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;

owenpan wrote:
> Can this be valid C++?
Yes! (https://godbolt.org/z/bnYahK4cz) (also I copied it from FormatTest)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147377

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

[PATCH] D147377: [clang-format] Don't allow variable decls to have trailing return arrows

[Emilia Dreamer via Phabricator via cfe-commits]

rymiel updated this revision to Diff 510317.
rymiel marked an inline comment as done.
rymiel edited the summary of this revision.
rymiel added a comment.

update commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147377

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1468,6 +1468,72 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
+  auto Tokens = annotate("auto f() -> int;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator->() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator++(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto foo() -> auto { return Val; }");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("struct S { auto bar() const -> int; };");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::arrow, TT_TrailingReturnArrow);
+
+  // Not trailing return arrows
+  Tokens = annotate("auto a = b->c;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = (b)->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b->c();");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("decltype(auto) a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b->c(); }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[11], tok::arrow, TT_Unknown);
+
+  // Mixed
+  Tokens = annotate("auto f() -> int { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+  EXPECT_TOKEN(Tokens[13], tok::arrow, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
 return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1919,7 +1919,7 @@
Style.Language == FormatStyle::LK_Java) {
   Current.setType(TT_LambdaArrow);
 } else if (Current.is(tok::arrow) && AutoFound &&
-   (Line.MustBeDeclaration || Line.InPPDirective) &&
+   (Line.MightBeFunctionDecl || Line.InPPDirective) &&
Current.NestingLevel == 0 &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1468,6 +1468,72 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
+  auto Tokens = annotate("auto f() -> int;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator->() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator++(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKE

[PATCH] D142688: [Clang][Driver] Handle LoongArch multiarch tuples

[WÁNG Xuěruì via Phabricator via cfe-commits]

xen0n updated this revision to Diff 510320.
xen0n added a comment.

Rebase and reflect the Toolchain Conventions change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142688

Files:
  clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/loongarch-abi.c

Index: clang/test/Driver/loongarch-abi.c
===
--- clang/test/Driver/loongarch-abi.c
+++ clang/test/Driver/loongarch-abi.c
@@ -16,6 +16,34 @@
 // RUN: %clang --target=loongarch64-unknown-elf %s -fsyntax-only -### -mabi=lp64d 2>&1 \
 // RUN:   | FileCheck --check-prefix=LP64D %s
 
+// RUN: %clang --target=loongarch32-linux-gnusf %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=ILP32S %s
+// RUN: %clang --target=loongarch32-linux-gnuf32 %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=ILP32F %s
+// RUN: %clang --target=loongarch32-linux-gnuf64 %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=ILP32D %s
+// RUN: %clang --target=loongarch32-linux-gnu %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=ILP32D %s
+
+// RUN: %clang --target=loongarch64-linux-gnusf %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=LP64S %s
+// RUN: %clang --target=loongarch64-linux-gnuf32 %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=LP64F %s
+// RUN: %clang --target=loongarch64-linux-gnuf64 %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=LP64D %s
+// RUN: %clang --target=loongarch64-linux-gnu %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=LP64D %s
+
+// Check that -mabi prevails in case of conflicts with the triple-implied ABI.
+// RUN: %clang --target=loongarch32-linux-gnuf64 %s -fsyntax-only -### -mabi=ilp32s 2>&1 \
+// RUN:   | FileCheck --check-prefix=ILP32S %s
+// RUN: %clang --target=loongarch64-linux-gnuf64 %s -fsyntax-only -### -mabi=lp64s 2>&1 \
+// RUN:   | FileCheck --check-prefix=LP64S %s
+// RUN: %clang --target=loongarch32-linux-gnu %s -fsyntax-only -### -mabi=ilp32s 2>&1 \
+// RUN:   | FileCheck --check-prefix=ILP32S %s
+// RUN: %clang --target=loongarch64-linux-gnu %s -fsyntax-only -### -mabi=lp64s 2>&1 \
+// RUN:   | FileCheck --check-prefix=LP64S %s
+
 // ILP32S: "-target-abi" "ilp32s"
 // ILP32F: "-target-abi" "ilp32f"
 // ILP32D: "-target-abi" "ilp32d"
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -86,6 +86,39 @@
   case llvm::Triple::aarch64_be:
 return "aarch64_be-linux-gnu";
 
+  case llvm::Triple::loongarch64: {
+const char *Libc;
+const char *FPFlavor;
+
+if (TargetTriple.isGNUEnvironment()) {
+  Libc = "gnu";
+} else if (TargetTriple.isMusl()) {
+  Libc = "musl";
+} else {
+  return TargetTriple.str();
+}
+
+switch (TargetEnvironment) {
+default:
+  return TargetTriple.str();
+case llvm::Triple::GNUSF:
+  FPFlavor = "sf";
+  break;
+case llvm::Triple::GNUF32:
+  FPFlavor = "f32";
+  break;
+case llvm::Triple::GNU:
+case llvm::Triple::GNUF64:
+  // This was going to be "f64" in an earlier Toolchain Conventions
+  // revision, but starting from Feb 2023 the F64 ABI variants are
+  // unmarked in their canonical forms.
+  FPFlavor = "";
+  break;
+}
+
+return std::string("loongarch64-linux-") + Libc + FPFlavor;
+  }
+
   case llvm::Triple::m68k:
 return "m68k-linux-gnu";
 
Index: clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
===
--- clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
+++ clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
@@ -54,7 +54,28 @@
   }
 
   // Choose a default based on the triple.
-  return IsLA32 ? "ilp32d" : "lp64d";
+  // Honor the explicit ABI modifier suffix in triple's environment part if
+  // present, falling back to {ILP32,LP64}D otherwise.
+  switch (Triple.getEnvironment()) {
+  case llvm::Triple::GNUSF:
+return IsLA32 ? "ilp32s" : "lp64s";
+  case llvm::Triple::GNUF32:
+return IsLA32 ? "ilp32f" : "lp64f";
+  case llvm::Triple::GNUF64:
+// This was originally permitted (and indeed the canonical way) to
+// represent the {ILP32,LP64}D ABIs, but in Feb 2023 Loongson decided to
+// drop the explicit suffix in favor of unmarked `-gnu` for the
+// "general-purpose" ABIs, among other non-technical reasons.
+//
+// The spec change did not mention whether existing usages of "gnuf64"
+// shall remain valid or not, so we are going to continue recognizing it
+// for some time, until it is clear that everyone else has migrated away
+// from it.
+[[fallthrough]];
+  case llvm::Triple::GNU:
+  default:
+return IsLA32 ? "il

[clang] aeee4eb - Stop modifying trailing return types.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2023-04-02T01:41:49-07:00
New Revision: aeee4ebd689171c963aa5d973e14cb6e731eb147

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

LOG: Stop modifying trailing return types.

This change reverts the functional change from D144626 but retains its
test. Instead of dealing with the possibility that a trailing requires
clause might have been rewritten into some other incorrect form, just
stop rewriting it.

No functionality changes intended.

Reviewed By: erichkeane, ChuanqiXu

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

Added: 


Modified: 
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/SemaConcept.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index bd4f2ff024949..74826da6703f5 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -6619,28 +6619,8 @@ bool ASTContext::isSameEntity(const NamedDecl *X, const 
NamedDecl *Y) const {
   return false;
 }
 
-// The trailing require clause of instantiated function may change during
-// the semantic analysis. Trying to get the primary template function (if
-// exists) to compare the primary trailing require clause.
-auto TryToGetPrimaryTemplatedFunction =
-[](const FunctionDecl *FD) -> const FunctionDecl * {
-  switch (FD->getTemplatedKind()) {
-  case FunctionDecl::TK_DependentNonTemplate:
-return FD->getInstantiatedFromDecl();
-  case FunctionDecl::TK_FunctionTemplate:
-return FD->getDescribedFunctionTemplate()->getTemplatedDecl();
-  case FunctionDecl::TK_MemberSpecialization:
-return FD->getInstantiatedFromMemberFunction();
-  case FunctionDecl::TK_FunctionTemplateSpecialization:
-return FD->getPrimaryTemplate()->getTemplatedDecl();
-  default:
-return FD;
-  }
-};
-const FunctionDecl *PrimaryX = TryToGetPrimaryTemplatedFunction(FuncX);
-const FunctionDecl *PrimaryY = TryToGetPrimaryTemplatedFunction(FuncY);
-if (!isSameConstraintExpr(PrimaryX->getTrailingRequiresClause(),
-  PrimaryY->getTrailingRequiresClause()))
+if (!isSameConstraintExpr(FuncX->getTrailingRequiresClause(),
+  FuncY->getTrailingRequiresClause()))
   return false;
 
 auto GetTypeAsWritten = [](const FunctionDecl *FD) {

diff  --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 4ff86843dacc1..2882b10613fdc 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -704,26 +704,10 @@ bool Sema::CheckFunctionConstraints(const FunctionDecl 
*FD,
 Record = const_cast(Method->getParent());
   }
   CXXThisScopeRAII ThisScope(*this, Record, ThisQuals, Record != nullptr);
-  // We substitute with empty arguments in order to rebuild the atomic
-  // constraint in a constant-evaluated context.
-  // FIXME: Should this be a dedicated TreeTransform?
-  const Expr *RC = FD->getTrailingRequiresClause();
-  llvm::SmallVector Converted;
-
-  if (CheckConstraintSatisfaction(
-  FD, {RC}, Converted, *MLTAL,
-  SourceRange(UsageLoc.isValid() ? UsageLoc : FD->getLocation()),
-  Satisfaction))
-return true;
-
-  // FIXME: we need to do this for the function constraints for
-  // comparison of constraints to work, but do we also need to do it for
-  // CheckInstantiatedFunctionConstraints?  That one is more 
diff icult, but we
-  // seem to always just pick up the constraints from the primary template.
-  assert(Converted.size() <= 1 && "Got more expressions converted?");
-  if (!Converted.empty() && Converted[0] != nullptr)
-const_cast(FD)->setTrailingRequiresClause(Converted[0]);
-  return false;
+  return CheckConstraintSatisfaction(
+  FD, {FD->getTrailingRequiresClause()}, *MLTAL,
+  SourceRange(UsageLoc.isValid() ? UsageLoc : FD->getLocation()),
+  Satisfaction);
 }
 
 



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

[PATCH] D147281: Stop modifying trailing return types.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaeee4ebd6891: Stop modifying trailing return types. 
(authored by rsmith).

Changed prior to commit:
  https://reviews.llvm.org/D147281?vs=509840&id=510321#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147281

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaConcept.cpp


Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -704,26 +704,10 @@
 Record = const_cast(Method->getParent());
   }
   CXXThisScopeRAII ThisScope(*this, Record, ThisQuals, Record != nullptr);
-  // We substitute with empty arguments in order to rebuild the atomic
-  // constraint in a constant-evaluated context.
-  // FIXME: Should this be a dedicated TreeTransform?
-  const Expr *RC = FD->getTrailingRequiresClause();
-  llvm::SmallVector Converted;
-
-  if (CheckConstraintSatisfaction(
-  FD, {RC}, Converted, *MLTAL,
-  SourceRange(UsageLoc.isValid() ? UsageLoc : FD->getLocation()),
-  Satisfaction))
-return true;
-
-  // FIXME: we need to do this for the function constraints for
-  // comparison of constraints to work, but do we also need to do it for
-  // CheckInstantiatedFunctionConstraints?  That one is more difficult, but we
-  // seem to always just pick up the constraints from the primary template.
-  assert(Converted.size() <= 1 && "Got more expressions converted?");
-  if (!Converted.empty() && Converted[0] != nullptr)
-const_cast(FD)->setTrailingRequiresClause(Converted[0]);
-  return false;
+  return CheckConstraintSatisfaction(
+  FD, {FD->getTrailingRequiresClause()}, *MLTAL,
+  SourceRange(UsageLoc.isValid() ? UsageLoc : FD->getLocation()),
+  Satisfaction);
 }
 
 
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6619,28 +6619,8 @@
   return false;
 }
 
-// The trailing require clause of instantiated function may change during
-// the semantic analysis. Trying to get the primary template function (if
-// exists) to compare the primary trailing require clause.
-auto TryToGetPrimaryTemplatedFunction =
-[](const FunctionDecl *FD) -> const FunctionDecl * {
-  switch (FD->getTemplatedKind()) {
-  case FunctionDecl::TK_DependentNonTemplate:
-return FD->getInstantiatedFromDecl();
-  case FunctionDecl::TK_FunctionTemplate:
-return FD->getDescribedFunctionTemplate()->getTemplatedDecl();
-  case FunctionDecl::TK_MemberSpecialization:
-return FD->getInstantiatedFromMemberFunction();
-  case FunctionDecl::TK_FunctionTemplateSpecialization:
-return FD->getPrimaryTemplate()->getTemplatedDecl();
-  default:
-return FD;
-  }
-};
-const FunctionDecl *PrimaryX = TryToGetPrimaryTemplatedFunction(FuncX);
-const FunctionDecl *PrimaryY = TryToGetPrimaryTemplatedFunction(FuncY);
-if (!isSameConstraintExpr(PrimaryX->getTrailingRequiresClause(),
-  PrimaryY->getTrailingRequiresClause()))
+if (!isSameConstraintExpr(FuncX->getTrailingRequiresClause(),
+  FuncY->getTrailingRequiresClause()))
   return false;
 
 auto GetTypeAsWritten = [](const FunctionDecl *FD) {


Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -704,26 +704,10 @@
 Record = const_cast(Method->getParent());
   }
   CXXThisScopeRAII ThisScope(*this, Record, ThisQuals, Record != nullptr);
-  // We substitute with empty arguments in order to rebuild the atomic
-  // constraint in a constant-evaluated context.
-  // FIXME: Should this be a dedicated TreeTransform?
-  const Expr *RC = FD->getTrailingRequiresClause();
-  llvm::SmallVector Converted;
-
-  if (CheckConstraintSatisfaction(
-  FD, {RC}, Converted, *MLTAL,
-  SourceRange(UsageLoc.isValid() ? UsageLoc : FD->getLocation()),
-  Satisfaction))
-return true;
-
-  // FIXME: we need to do this for the function constraints for
-  // comparison of constraints to work, but do we also need to do it for
-  // CheckInstantiatedFunctionConstraints?  That one is more difficult, but we
-  // seem to always just pick up the constraints from the primary template.
-  assert(Converted.size() <= 1 && "Got more expressions converted?");
-  if (!Converted.empty() && Converted[0] != nullptr)
-const_cast(FD)->setTrailingRequiresClause(Converted[0]);
-  return false;
+  return CheckConstraintSatisfaction(
+  FD, {FD->getTrailingRequiresClause()}, *MLTAL,
+  SourceRange(UsageLoc.isValid() ? UsageLoc : FD->getLocatio

[PATCH] D142688: [Clang][Driver] Handle LoongArch multiarch tuples

[WÁNG Xuěruì via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Arch/LoongArch.cpp:66
+  default:
+return IsLA32 ? "ilp32d" : "lp64d";
+  }

xry111 wrote:
> SixWeining wrote:
> > Better to be `f`? (Probably most 32-bit hardwares do not support 
> > double-float? But I'm not sure about this.)
> The ISA manual says 64-bit float support is not limited by 32-bit basic ISA 
> support (except the moving instructions accessing 64-bit GPR).
AFAIK the consensus (even inside Loongson) is that even LA32's unmarked/default 
ABI is going to have hard F64 available. And yes the ISA manual is consistent 
with this view too. (IMO LA32 should instead default to soft-float, but if 
Loongson's commercial roadmap can guarantee hardware FPU availability on all 
meaningful LA32 models then that would backfire, who knows...)



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:41
 /// so we provide a rough mapping here.
 std::string Linux::getMultiarchTriple(const Driver &D,
   const llvm::Triple &TargetTriple,

SixWeining wrote:
> xen0n wrote:
> > SixWeining wrote:
> > > Missing test. Perhaps add some in `clang/test/Driver/linux-ld.c` and 
> > > `clang/test/Driver/linux-header-search.cpp`? Or postpone this change 
> > > until loongarch is upstreamed to Debian?
> > Thanks, I'll try adding the tests in the next revision.
> > 
> > But given [[ https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1028654 | 
> > the recent reversal of upstream dpkg support ]] (apparently due to 
> > miscommunication) it may be prudent to wait until a consensus is reached 
> > Debian-side.
> [[ https://git.dpkg.org/cgit/dpkg/dpkg.git/commit/?id=9dfffbbae | The 
> multiarch tuples have been upstreamed to dpkg ]].  I think we can amend the 
> change and move on now.
Sorry for the delay in processing this, I've updated the code to reflect the 
latest spec change. I'll still have to check the unit-test part though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142688

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

[PATCH] D146042: [clang-format] Fix numerous issues with "LambdaBodyIndentation: OuterScope" option

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D146042#4204651 , @jp4a50 wrote:

> I'm confident that the patch will indent all those examples correctly when 
> they are at block scope which is the only place those snippets will actually 
> be valid code. I added an exception (see comment in ContinuationIndenter.cpp) 
> to the code to disable `OuterScope`'s behaviour when the line's indentation 
> level is 0 (i.e. statements at namespace scope) because otherwise you can end 
> up with things like:
>
>   Namespace::Foo::Foo(
> Arg arg1, Arg arg2,
> Arg arg3, Arg arg4)
> : init1{arg1},
>   init2{[arg2]() {
> return arg2;
>   }},
>   init3{arg3},
>   init4{arg4} {}

Sorry that I missed it.




Comment at: clang/include/clang/Format/Format.h:2652-2656
   /// containing the lambda signature. For callback-heavy code, it may improve
   /// readability to have the signature indented two levels and to use
   /// ``OuterScope``. The KJ style guide requires ``OuterScope``.
   /// `KJ style guide
   /// `_

We might as well clean it up a bit.



Comment at: clang/unittests/Format/FormatTest.cpp:21997
+   "  })));\n"
+   "}\n",
+   Style);

Ditto below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146042

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

[PATCH] D142688: [Clang][Driver] Handle LoongArch multiarch tuples

[WÁNG Xuěruì via Phabricator via cfe-commits]

xen0n updated this revision to Diff 510333.
xen0n marked an inline comment as done.
xen0n added a comment.

Rebase and add tests, also use Twine for concatenating strings


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142688

Files:
  clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/Inputs/debian_loong64_tree/usr/include/c++/13/backward/.keep
  
clang/test/Driver/Inputs/debian_loong64_tree/usr/include/loongarch64-linux-gnu/c++/.keep
  
clang/test/Driver/Inputs/debian_loong64_tree/usr/lib/gcc/loongarch64-linux-gnu/13/crtbegin.o
  
clang/test/Driver/Inputs/debian_loong64_tree/usr/lib/gcc/loongarch64-linux-gnu/13/crtend.o
  
clang/test/Driver/Inputs/debian_loong64_tree/usr/lib/gcc/loongarch64-linux-gnu/13/include/.keep
  
clang/test/Driver/Inputs/debian_loong64_tree/usr/lib/loongarch64-linux-gnu/crt1.o
  
clang/test/Driver/Inputs/debian_loong64_tree/usr/lib/loongarch64-linux-gnu/crti.o
  
clang/test/Driver/Inputs/debian_loong64_tree/usr/lib/loongarch64-linux-gnu/crtn.o
  clang/test/Driver/linux-header-search.cpp
  clang/test/Driver/linux-ld.c
  clang/test/Driver/loongarch-abi.c

Index: clang/test/Driver/loongarch-abi.c
===
--- clang/test/Driver/loongarch-abi.c
+++ clang/test/Driver/loongarch-abi.c
@@ -16,6 +16,34 @@
 // RUN: %clang --target=loongarch64-unknown-elf %s -fsyntax-only -### -mabi=lp64d 2>&1 \
 // RUN:   | FileCheck --check-prefix=LP64D %s
 
+// RUN: %clang --target=loongarch32-linux-gnusf %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=ILP32S %s
+// RUN: %clang --target=loongarch32-linux-gnuf32 %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=ILP32F %s
+// RUN: %clang --target=loongarch32-linux-gnuf64 %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=ILP32D %s
+// RUN: %clang --target=loongarch32-linux-gnu %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=ILP32D %s
+
+// RUN: %clang --target=loongarch64-linux-gnusf %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=LP64S %s
+// RUN: %clang --target=loongarch64-linux-gnuf32 %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=LP64F %s
+// RUN: %clang --target=loongarch64-linux-gnuf64 %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=LP64D %s
+// RUN: %clang --target=loongarch64-linux-gnu %s -fsyntax-only -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=LP64D %s
+
+// Check that -mabi prevails in case of conflicts with the triple-implied ABI.
+// RUN: %clang --target=loongarch32-linux-gnuf64 %s -fsyntax-only -### -mabi=ilp32s 2>&1 \
+// RUN:   | FileCheck --check-prefix=ILP32S %s
+// RUN: %clang --target=loongarch64-linux-gnuf64 %s -fsyntax-only -### -mabi=lp64s 2>&1 \
+// RUN:   | FileCheck --check-prefix=LP64S %s
+// RUN: %clang --target=loongarch32-linux-gnu %s -fsyntax-only -### -mabi=ilp32s 2>&1 \
+// RUN:   | FileCheck --check-prefix=ILP32S %s
+// RUN: %clang --target=loongarch64-linux-gnu %s -fsyntax-only -### -mabi=lp64s 2>&1 \
+// RUN:   | FileCheck --check-prefix=LP64S %s
+
 // ILP32S: "-target-abi" "ilp32s"
 // ILP32F: "-target-abi" "ilp32f"
 // ILP32D: "-target-abi" "ilp32d"
Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -830,6 +830,30 @@
 // CHECK-ARM-HF: "-dynamic-linker" "{{.*}}/lib/ld-linux-armhf.so.3"
 //
 // RUN: %clang -### %s -no-pie 2>&1 \
+// RUN: --target=loongarch64-linux-gnu \
+// RUN:   | FileCheck --check-prefix=CHECK-LOONGARCH-LP64D %s
+// RUN: %clang -### %s -no-pie 2>&1 \
+// RUN: --target=loongarch64-linux-gnuf64 \
+// RUN:   | FileCheck --check-prefix=CHECK-LOONGARCH-LP64D %s
+// CHECK-LOONGARCH-LP64D: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LOONGARCH-LP64D: "-m" "elf64loongarch"
+// CHECK-LOONGARCH-LP64D: "-dynamic-linker" "{{.*}}/lib64/ld-linux-loongarch-lp64d.so.1"
+//
+// RUN: %clang -### %s -no-pie 2>&1 \
+// RUN: --target=loongarch64-linux-gnuf32 \
+// RUN:   | FileCheck --check-prefix=CHECK-LOONGARCH-LP64F %s
+// CHECK-LOONGARCH-LP64F: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LOONGARCH-LP64F: "-m" "elf64loongarch"
+// CHECK-LOONGARCH-LP64F: "-dynamic-linker" "{{.*}}/lib64/ld-linux-loongarch-lp64f.so.1"
+//
+// RUN: %clang -### %s -no-pie 2>&1 \
+// RUN: --target=loongarch64-linux-gnusf \
+// RUN:   | FileCheck --check-prefix=CHECK-LOONGARCH-LP64S %s
+// CHECK-LOONGARCH-LP64S: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LOONGARCH-LP64S: "-m" "elf64loongarch"
+// CHECK-LOONGARCH-LP64S: "-dynamic-linker" "{{.*}}/lib64/ld-linux-loongarch-lp64s.so.1"
+//
+// RUN: %clang -### %s -no-pie 2>&1 \
 // RUN: --target=powerpc64-linux-gnu \
 // RUN:   | FileCheck --check-prefix=CHECK-PPC64 %s
 // CHECK-PPC64: "{{.*}}ld{{(.exe)?}}"
@@ -1311,6 +1335,29 @@
 // RUN:   | FileCheck --check-pref

[PATCH] D142688: [Clang][Driver] Handle LoongArch multiarch tuples

[WÁNG Xuěruì via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:41
 /// so we provide a rough mapping here.
 std::string Linux::getMultiarchTriple(const Driver &D,
   const llvm::Triple &TargetTriple,

xen0n wrote:
> SixWeining wrote:
> > xen0n wrote:
> > > SixWeining wrote:
> > > > Missing test. Perhaps add some in `clang/test/Driver/linux-ld.c` and 
> > > > `clang/test/Driver/linux-header-search.cpp`? Or postpone this change 
> > > > until loongarch is upstreamed to Debian?
> > > Thanks, I'll try adding the tests in the next revision.
> > > 
> > > But given [[ https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1028654 | 
> > > the recent reversal of upstream dpkg support ]] (apparently due to 
> > > miscommunication) it may be prudent to wait until a consensus is reached 
> > > Debian-side.
> > [[ https://git.dpkg.org/cgit/dpkg/dpkg.git/commit/?id=9dfffbbae | The 
> > multiarch tuples have been upstreamed to dpkg ]].  I think we can amend the 
> > change and move on now.
> Sorry for the delay in processing this, I've updated the code to reflect the 
> latest spec change. I'll still have to check the unit-test part though.
I've now added tests for the Debian sysroot layout, based on Revy's latest work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142688

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

[PATCH] D147411: [clang-tidy] Fix readability-static-accessed-through-instance check for anonymous structs

[André Schackier via Phabricator via cfe-commits]

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

Previously we would provide a fixit which looked like
this `unnamed struct at ...::f()` but which is obviously
not valid C/C++.

Since there is no real nice way to accesses a static function
from an anonymous struct anyways we simply ignore all
anonymous structs.

Fixes llvm#61736


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147411

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


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp
@@ -285,3 +285,27 @@
 // CHECK-MESSAGES-NOT: :[[@LINE-1]]:10: warning: static member
 
 } // namespace Bugzilla_48758
+
+// https://github.com/llvm/llvm-project/issues/61736
+namespace llvm_issue_61736
+{
+
+struct {
+  static void f() {}
+} AnonStruct, *AnonStructPointer;
+
+class {
+  public:
+  static void f() {}
+} AnonClass, *AnonClassPointer;
+
+void testAnonymousStructAndClass()
+{
+  AnonStruct.f();
+  AnonStructPointer->f();
+
+  AnonClass.f();
+  AnonClassPointer->f();
+}
+
+} // namespace llvm_issue_61736
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -267,6 +267,10 @@
   string for ``Prefix`` or ``Suffix`` options could result in the style not
   being used.
 
+- Fixed an issue in :doc:`readability-static-accessed-through-instance
+  ` when using
+  anonymous structs or classes.
+
 - Fixed an issue in 
:doc:`google-readability-avoid-underscore-in-googletest-name
   ` 
when using
   ``DISABLED_`` in the test suite name.
Index: 
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
+++ 
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
@@ -74,6 +74,10 @@
   std::string BaseTypeName =
   BaseType.getAsString(PrintingPolicyWithSupressedTag);
 
+  // Ignore anonymous structs/classes
+  if (StringRef(BaseTypeName).contains("(unnamed "))
+return;
+
   // Do not warn for CUDA built-in variables.
   if (StringRef(BaseTypeName).startswith("__cuda_builtin_"))
 return;


Index: clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp
@@ -285,3 +285,27 @@
 // CHECK-MESSAGES-NOT: :[[@LINE-1]]:10: warning: static member
 
 } // namespace Bugzilla_48758
+
+// https://github.com/llvm/llvm-project/issues/61736
+namespace llvm_issue_61736
+{
+
+struct {
+  static void f() {}
+} AnonStruct, *AnonStructPointer;
+
+class {
+  public:
+  static void f() {}
+} AnonClass, *AnonClassPointer;
+
+void testAnonymousStructAndClass()
+{
+  AnonStruct.f();
+  AnonStructPointer->f();
+
+  AnonClass.f();
+  AnonClassPointer->f();
+}
+
+} // namespace llvm_issue_61736
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -267,6 +267,10 @@
   string for ``Prefix`` or ``Suffix`` options could result in the style not
   being used.
 
+- Fixed an issue in :doc:`readability-static-accessed-through-instance
+  ` when using
+  anonymous structs or classes.
+
 - Fixed an issue in :doc:`google-readability-avoid-underscore-in-googletest-name
   ` when using
   ``DISABLED_`` in the test suite name.
Index: clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
@@ -74,6 +74,10 @@
   std::string BaseTypeName =
   BaseType.getAsString(PrintingPolicyWithSupressedTag);
 
+  // Ignore anonymous structs/classes
+  if (StringRef(BaseTypeName).contains("(unname

[PATCH] D147411: [clang-tidy] Fix readability-static-accessed-through-instance check for anonymous structs

[André Schackier via Phabricator via cfe-commits]

AMS21 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp:78
+  // Ignore anonymous structs/classes
+  if (StringRef(BaseTypeName).contains("(unnamed "))
+return;

I wonder if there's a cleaner and more performant way to check this.
I've tried `isAnonymousStructOrUnion()` from the `RecordDecl` but that seems to 
be about something else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147411

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

[clang] 806b74b - [HIP] clang should pass `-mno-amdgpu-ieee` to -cc1

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2023-04-02T08:11:17-04:00
New Revision: 806b74b0de6e7354033ec36c833d43ca8ff13fa7

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

LOG: [HIP] clang should pass `-mno-amdgpu-ieee` to -cc1

Reviewed by: Artem Belevich

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/hip-options.hip

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 1984e1393a15c..41fe89337d1b1 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7146,6 +7146,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
 
 Args.addOptInFlag(CmdArgs, options::OPT_munsafe_fp_atomics,
   options::OPT_mno_unsafe_fp_atomics);
+Args.addOptOutFlag(CmdArgs, options::OPT_mamdgpu_ieee,
+   options::OPT_mno_amdgpu_ieee);
   }
 
   // For all the host OpenMP offloading compile jobs we need to pass the 
targets

diff  --git a/clang/test/Driver/hip-options.hip 
b/clang/test/Driver/hip-options.hip
index 97d0c1370bc42..0bce2ace10a49 100644
--- a/clang/test/Driver/hip-options.hip
+++ b/clang/test/Driver/hip-options.hip
@@ -126,3 +126,15 @@
 // RUN:   --offload-arch=gfx906 %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=KANNEG %s
 // KANNEG-NOT: "-fhip-kernel-arg-name"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mno-amdgpu-ieee -mamdgpu-ieee \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=IEEE-ON %s
+// IEEE-ON-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-m{{(no-)?}}amdgpu-ieee"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mamdgpu-ieee -mno-amdgpu-ieee 
-ffast-math \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefixes=IEEE-OFF 
%s
+// IEEE-OFF: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-mno-amdgpu-ieee"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mamdgpu-ieee -mno-amdgpu-ieee 
-ffast-math \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck 
-check-prefixes=IEEE-OFF-NEG %s
+// IEEE-OFF-NEG-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-mamdgpu-ieee"



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

[PATCH] D145721: [HIP] clang should pass `-mno-amdgpu-ieee` to -cc1

[Yaxun Liu 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 rG806b74b0de6e: [HIP] clang should pass `-mno-amdgpu-ieee` to 
-cc1 (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145721

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/hip-options.hip


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -126,3 +126,15 @@
 // RUN:   --offload-arch=gfx906 %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=KANNEG %s
 // KANNEG-NOT: "-fhip-kernel-arg-name"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mno-amdgpu-ieee -mamdgpu-ieee \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=IEEE-ON %s
+// IEEE-ON-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-m{{(no-)?}}amdgpu-ieee"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mamdgpu-ieee -mno-amdgpu-ieee 
-ffast-math \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefixes=IEEE-OFF 
%s
+// IEEE-OFF: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-mno-amdgpu-ieee"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mamdgpu-ieee -mno-amdgpu-ieee 
-ffast-math \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck 
-check-prefixes=IEEE-OFF-NEG %s
+// IEEE-OFF-NEG-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-mamdgpu-ieee"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7146,6 +7146,8 @@
 
 Args.addOptInFlag(CmdArgs, options::OPT_munsafe_fp_atomics,
   options::OPT_mno_unsafe_fp_atomics);
+Args.addOptOutFlag(CmdArgs, options::OPT_mamdgpu_ieee,
+   options::OPT_mno_amdgpu_ieee);
   }
 
   // For all the host OpenMP offloading compile jobs we need to pass the 
targets


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -126,3 +126,15 @@
 // RUN:   --offload-arch=gfx906 %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=KANNEG %s
 // KANNEG-NOT: "-fhip-kernel-arg-name"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mno-amdgpu-ieee -mamdgpu-ieee \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=IEEE-ON %s
+// IEEE-ON-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-m{{(no-)?}}amdgpu-ieee"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mamdgpu-ieee -mno-amdgpu-ieee -ffast-math \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefixes=IEEE-OFF %s
+// IEEE-OFF: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-mno-amdgpu-ieee"
+
+// RUN: %clang -### -nogpuinc -nogpulib -mamdgpu-ieee -mno-amdgpu-ieee -ffast-math \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefixes=IEEE-OFF-NEG %s
+// IEEE-OFF-NEG-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-mamdgpu-ieee"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7146,6 +7146,8 @@
 
 Args.addOptInFlag(CmdArgs, options::OPT_munsafe_fp_atomics,
   options::OPT_mno_unsafe_fp_atomics);
+Args.addOptOutFlag(CmdArgs, options::OPT_mamdgpu_ieee,
+   options::OPT_mno_amdgpu_ieee);
   }
 
   // For all the host OpenMP offloading compile jobs we need to pass the targets
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

[Andrey Ali Khan Bolshakov via Phabricator via cfe-commits]

bolshakov-a updated this revision to Diff 510343.
bolshakov-a edited the summary of this revision.
bolshakov-a added a comment.

Mangle array subscripts and base class members in references to subobjects.


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

https://reviews.llvm.org/D146386

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-ms-templates.cpp

Index: clang/test/CodeGenCXX/mangle-ms-templates.cpp
===
--- clang/test/CodeGenCXX/mangle-ms-templates.cpp
+++ clang/test/CodeGenCXX/mangle-ms-templates.cpp
@@ -272,7 +272,7 @@
 };
 extern const record inst;
 void recref(type1) {}
-// CHECK: "?recref@@YAXU?$type1@$E?inst@@3Urecord@@B@@@Z"
+// CHECK: "?recref@@YAXU?$type1@$1?inst@@3Urecord@@B@@@Z"
 
 struct _GUID {};
 struct __declspec(uuid("{12345678-1234-1234-1234-1234567890aB}")) uuid;
@@ -286,7 +286,7 @@
 void fun(UUIDType1 a) {}
 // CHECK: "?fun@@YAXU?$UUIDType1@Uuuid@@$1?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
 void fun(UUIDType2 b) {}
-// CHECK: "?fun@@YAXU?$UUIDType2@Uuuid@@$E?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
+// CHECK: "?fun@@YAXU?$UUIDType2@Uuuid@@$1?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
 
 template  struct TypeWithFriendDefinition {
   friend void FunctionDefinedWithInjectedName(TypeWithFriendDefinition) {}
Index: clang/test/CodeGenCXX/mangle-class-nttp.cpp
===
--- clang/test/CodeGenCXX/mangle-class-nttp.cpp
+++ clang/test/CodeGenCXX/mangle-class-nttp.cpp
@@ -15,7 +15,7 @@
 
 int n = 0;
 // CHECK: define weak_odr void @_Z1fIXtl1BadL_Z1nvv(
-// MSABI: define {{.*}} @"??$f@$2UB@@PEBH1?n@@3HAH0AYAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UB@@PEBHE?n@@3HAH0AYAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1BLPKi0ELi1vv(
 // MSABI: define {{.*}} @"??$f@$2UB@@PEBH0A@H00@@@YAXXZ"
@@ -36,15 +36,19 @@
 
 // Pointers to subobjects.
 struct Nested { union { int k; int arr[2]; }; } nested[2];
-struct Derived : A, Nested { int z; } extern derived;
+struct Derived : A, Nested { int z; A a_field; } extern derived;
 // CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z7derivedE16vv
 // MSABI: define {{.*}} void @"??$f@$2UB@@PEBH56E?derived@@3UDerived@@Az@@@H0AYAXXZ"
 template void f();
-// FIXME: We don't know the MS ABI mangling for array subscripting and
-// past-the-end pointers yet.
-#ifndef _WIN32
+// CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z7derivedE20vv
+// MSABI: define {{.*}} void @"??$f@$2UB@@PEBH566E?derived@@3UDerived@@Aa_field@@a@@@H0AYAXXZ"
+template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z6nestedE_vv
+// MSABI: define {{.*}} void @"??$f@$2UB@@PEBH56CE?nested@@3PAUNested@@A0A@@k@@@H0AYAXXZ"
 template void f();
+// Mangling of pointers to nested array elements and past-the-end pointers
+// is still incorrect in MSVC.
+#ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z6nestedE16_0pvv
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z7derivedE8pvv
@@ -59,14 +63,16 @@
 // CHECK: define weak_odr void @_Z1fIXtl2BRsoKiL_Z7derivedE16vv
 // MSABI: define {{.*}} void @"??$f@$2UBR@@AEBH6E?derived@@3UDerived@@Az@YAXXZ"
 template void f();
-// FIXME: We don't know the MS ABI mangling for array subscripting yet.
-#ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl2BRsoKiL_Z6nestedE_vv
+// MSABI: define {{.*}} void @"??$f@$2UBR@@AEBH6CE?nested@@3PAUNested@@A0A@@k@YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl2BRsoKiL_Z6nestedE12_0vv
+// MSABI: define {{.*}} void @"??$f@$2UBR@@AEBHC6CE?nested@@3PAUNested@@A00@arr@@00YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl2BRsoKiL_Z7derivedE4vv
+// MSABI: define {{.*}} void @"??$f@$2UBR@@AEBH66E?derived@@3UDerived@@AA@@b@YAXXZ"
 template void f();
+#ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl2BRdecvPKiplcvPcadL_Z7derivedELl16vv
 template void f();
 #endif
@@ -77,42 +83,93 @@
 // CHECK: define weak_odr void @_Z1fIXtl1CadsoKiL_Z7derivedE16vv
 // MSABI: define {{.*}} void @"??$f@$2UC@@PEBH56E?derived@@3UDerived@@Az@@YAXXZ"
 template void f();
-#ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl1CadsoKiL_Z7derivedE4vv
+// MSABI: define {{.*}} void @"??$f@$2UC@@PEBH566E?derived@@3UDerived@@AA@@b@@YAXXZ"
 template void f();
-#endif
 
 // Pointers to members.
 struct D { const int Derived::*p; int k; };
 template void f() {}
 // CHECK: define weak_odr void @_Z1fIXtl1DLM7DerivedKi0ELi1vv
-// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H0?0H00@@@YAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@HNH00@@@YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1DEEEvv
-// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H0?0H0AY

[clang] 6aa74ae - [HIP] Supports env var HIP_PATH

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2023-04-02T08:46:19-04:00
New Revision: 6aa74ae29ffc01b486798552672171f5a622fc2a

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

LOG: [HIP] Supports env var HIP_PATH

Currently HIP toolchain recognize env var ROCM_PATH and option --rocm-path
but only recognize --hip-path.

Some package management tools e.g. Spack relies on env var HIP_PATH to
be able to load different version of HIP dynamically.
(https://github.com/spack/spack/blob/develop/var/spack/repos/builtin/packages/hip/package.py#L446)
Therefore add support of env var HIP_PATH.

Reviewed by: Artem Belevich, Fangrui Song

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/AMDGPU.cpp
clang/test/Driver/rocm-detect.hip

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp 
b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index 99e2fb839221..acedcfabefe1 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -437,7 +437,11 @@ void RocmInstallationDetector::detectHIPRuntime() {
   SmallVector HIPSearchDirs;
   if (!HIPPathArg.empty())
 HIPSearchDirs.emplace_back(HIPPathArg.str(), /*StrictChecking=*/true);
-  else
+  else if (std::optional HIPPathEnv =
+   llvm::sys::Process::GetEnv("HIP_PATH")) {
+if (!HIPPathEnv->empty())
+  HIPSearchDirs.emplace_back(std::move(*HIPPathEnv));
+  } else
 HIPSearchDirs.append(getInstallationPathCandidates());
   auto &FS = D.getVFS();
 

diff  --git a/clang/test/Driver/rocm-detect.hip 
b/clang/test/Driver/rocm-detect.hip
index 01837bb45065..fbb4bbc25e3c 100644
--- a/clang/test/Driver/rocm-detect.hip
+++ b/clang/test/Driver/rocm-detect.hip
@@ -25,6 +25,31 @@
 // RUN:   --print-rocm-search-dirs %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=ROCM-ENV %s
 
+// Test interaction between environment variables HIP_PATH and ROCM_PATH.
+// Device libs are found under ROCM_PATH. HIP include files and HIP runtime 
library
+// are found under HIP_PATH.
+
+// RUN: rm -rf %t/myhip
+// RUN: mkdir -p %t/myhip
+// RUN: cp -r %S/Inputs/rocm/bin %t/myhip
+// RUN: env ROCM_PATH=%S/Inputs/rocm HIP_PATH=%t/myhip \
+// RUN:   %clang -### -target x86_64-linux-gnu --offload-arch=gfx1010 
--hip-link \
+// RUN:   --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-ENV,HIP-PATH %s
+
+// Test --hip-path option overrides environment variable HIP_PATH.
+
+// RUN: rm -rf %t/myhip
+// RUN: rm -rf %t/myhip_nouse
+// RUN: mkdir -p %t/myhip
+// RUN: mkdir -p %t/myhip_nouse
+// RUN: cp -r %S/Inputs/rocm/bin %t/myhip
+// RUN: cp -r %S/Inputs/rocm/bin %t/myhip_nouse
+// RUN: env ROCM_PATH=%S/Inputs/rocm HIP_PATH=%t/myhip_nouse \
+// RUN:   %clang -### -target x86_64-linux-gnu --offload-arch=gfx1010 
--hip-link \
+// RUN:   --hip-path=%t/myhip --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-ENV,HIP-PATH %s
+
 // Test detecting latest /opt/rocm-{release} directory.
 // RUN: rm -rf %T/opt
 // RUN: mkdir -p %T/opt
@@ -75,7 +100,11 @@
 
 // COMMON: "-triple" "amdgcn-amd-amdhsa"
 
-// ROCM-ENV: ROCm installation search path: {{.*}}/Inputs/rocm
+// ROCM-ENV: ROCm installation search path: [[ROCM_PATH:.*/Inputs/rocm]]
+
+// HIP-PATH: "-mlink-builtin-bitcode" 
"[[ROCM_PATH]]/amdgcn/bitcode/oclc_isa_version_1010.bc"
+// HIP-PATH: "-idirafter" "[[HIP_PATH:.*/myhip]]/include"
+// HIP-PATH: "-L[[HIP_PATH]]/lib" "-lamdhip64"
 
 // ROCM-REL: ROCm installation search path: {{.*}}/opt/rocm
 // ROCM-REL: ROCm installation search path: {{.*}}/opt/rocm-3.10.0



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

[PATCH] D145391: [HIP] Supports env var HIP_PATH

[Yaxun Liu 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 rG6aa74ae29ffc: [HIP] Supports env var HIP_PATH (authored by 
yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D145391?vs=505133&id=510344#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145391

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/test/Driver/rocm-detect.hip


Index: clang/test/Driver/rocm-detect.hip
===
--- clang/test/Driver/rocm-detect.hip
+++ clang/test/Driver/rocm-detect.hip
@@ -25,6 +25,31 @@
 // RUN:   --print-rocm-search-dirs %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=ROCM-ENV %s
 
+// Test interaction between environment variables HIP_PATH and ROCM_PATH.
+// Device libs are found under ROCM_PATH. HIP include files and HIP runtime 
library
+// are found under HIP_PATH.
+
+// RUN: rm -rf %t/myhip
+// RUN: mkdir -p %t/myhip
+// RUN: cp -r %S/Inputs/rocm/bin %t/myhip
+// RUN: env ROCM_PATH=%S/Inputs/rocm HIP_PATH=%t/myhip \
+// RUN:   %clang -### -target x86_64-linux-gnu --offload-arch=gfx1010 
--hip-link \
+// RUN:   --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-ENV,HIP-PATH %s
+
+// Test --hip-path option overrides environment variable HIP_PATH.
+
+// RUN: rm -rf %t/myhip
+// RUN: rm -rf %t/myhip_nouse
+// RUN: mkdir -p %t/myhip
+// RUN: mkdir -p %t/myhip_nouse
+// RUN: cp -r %S/Inputs/rocm/bin %t/myhip
+// RUN: cp -r %S/Inputs/rocm/bin %t/myhip_nouse
+// RUN: env ROCM_PATH=%S/Inputs/rocm HIP_PATH=%t/myhip_nouse \
+// RUN:   %clang -### -target x86_64-linux-gnu --offload-arch=gfx1010 
--hip-link \
+// RUN:   --hip-path=%t/myhip --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-ENV,HIP-PATH %s
+
 // Test detecting latest /opt/rocm-{release} directory.
 // RUN: rm -rf %T/opt
 // RUN: mkdir -p %T/opt
@@ -75,7 +100,11 @@
 
 // COMMON: "-triple" "amdgcn-amd-amdhsa"
 
-// ROCM-ENV: ROCm installation search path: {{.*}}/Inputs/rocm
+// ROCM-ENV: ROCm installation search path: [[ROCM_PATH:.*/Inputs/rocm]]
+
+// HIP-PATH: "-mlink-builtin-bitcode" 
"[[ROCM_PATH]]/amdgcn/bitcode/oclc_isa_version_1010.bc"
+// HIP-PATH: "-idirafter" "[[HIP_PATH:.*/myhip]]/include"
+// HIP-PATH: "-L[[HIP_PATH]]/lib" "-lamdhip64"
 
 // ROCM-REL: ROCm installation search path: {{.*}}/opt/rocm
 // ROCM-REL: ROCm installation search path: {{.*}}/opt/rocm-3.10.0
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -437,7 +437,11 @@
   SmallVector HIPSearchDirs;
   if (!HIPPathArg.empty())
 HIPSearchDirs.emplace_back(HIPPathArg.str(), /*StrictChecking=*/true);
-  else
+  else if (std::optional HIPPathEnv =
+   llvm::sys::Process::GetEnv("HIP_PATH")) {
+if (!HIPPathEnv->empty())
+  HIPSearchDirs.emplace_back(std::move(*HIPPathEnv));
+  } else
 HIPSearchDirs.append(getInstallationPathCandidates());
   auto &FS = D.getVFS();
 


Index: clang/test/Driver/rocm-detect.hip
===
--- clang/test/Driver/rocm-detect.hip
+++ clang/test/Driver/rocm-detect.hip
@@ -25,6 +25,31 @@
 // RUN:   --print-rocm-search-dirs %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=ROCM-ENV %s
 
+// Test interaction between environment variables HIP_PATH and ROCM_PATH.
+// Device libs are found under ROCM_PATH. HIP include files and HIP runtime library
+// are found under HIP_PATH.
+
+// RUN: rm -rf %t/myhip
+// RUN: mkdir -p %t/myhip
+// RUN: cp -r %S/Inputs/rocm/bin %t/myhip
+// RUN: env ROCM_PATH=%S/Inputs/rocm HIP_PATH=%t/myhip \
+// RUN:   %clang -### -target x86_64-linux-gnu --offload-arch=gfx1010 --hip-link \
+// RUN:   --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-ENV,HIP-PATH %s
+
+// Test --hip-path option overrides environment variable HIP_PATH.
+
+// RUN: rm -rf %t/myhip
+// RUN: rm -rf %t/myhip_nouse
+// RUN: mkdir -p %t/myhip
+// RUN: mkdir -p %t/myhip_nouse
+// RUN: cp -r %S/Inputs/rocm/bin %t/myhip
+// RUN: cp -r %S/Inputs/rocm/bin %t/myhip_nouse
+// RUN: env ROCM_PATH=%S/Inputs/rocm HIP_PATH=%t/myhip_nouse \
+// RUN:   %clang -### -target x86_64-linux-gnu --offload-arch=gfx1010 --hip-link \
+// RUN:   --hip-path=%t/myhip --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-ENV,HIP-PATH %s
+
 // Test detecting latest /opt/rocm-{release} directory.
 // RUN: rm -rf %T/opt
 // RUN: mkdir -p %T/opt
@@ -75,7 +100,11 @@
 
 // COMMON: "-triple" "amdgcn-amd-amdhsa"
 
-// ROCM-ENV: ROCm installation search path: {{.*}}/Inputs/rocm
+// ROCM-ENV: ROCm installation search path: [[ROCM_PATH:.*/Inputs/rocm]]
+
+// HIP-PATH: "-mlink-builtin-bitcode" "[

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

[Andrey Ali Khan Bolshakov via Phabricator via cfe-commits]

bolshakov-a updated this revision to Diff 510345.
bolshakov-a added a comment.

Minor refactoring. Don't hide function parameter with local variable.


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

https://reviews.llvm.org/D146386

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-ms-templates.cpp

Index: clang/test/CodeGenCXX/mangle-ms-templates.cpp
===
--- clang/test/CodeGenCXX/mangle-ms-templates.cpp
+++ clang/test/CodeGenCXX/mangle-ms-templates.cpp
@@ -272,7 +272,7 @@
 };
 extern const record inst;
 void recref(type1) {}
-// CHECK: "?recref@@YAXU?$type1@$E?inst@@3Urecord@@B@@@Z"
+// CHECK: "?recref@@YAXU?$type1@$1?inst@@3Urecord@@B@@@Z"
 
 struct _GUID {};
 struct __declspec(uuid("{12345678-1234-1234-1234-1234567890aB}")) uuid;
@@ -286,7 +286,7 @@
 void fun(UUIDType1 a) {}
 // CHECK: "?fun@@YAXU?$UUIDType1@Uuuid@@$1?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
 void fun(UUIDType2 b) {}
-// CHECK: "?fun@@YAXU?$UUIDType2@Uuuid@@$E?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
+// CHECK: "?fun@@YAXU?$UUIDType2@Uuuid@@$1?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
 
 template  struct TypeWithFriendDefinition {
   friend void FunctionDefinedWithInjectedName(TypeWithFriendDefinition) {}
Index: clang/test/CodeGenCXX/mangle-class-nttp.cpp
===
--- clang/test/CodeGenCXX/mangle-class-nttp.cpp
+++ clang/test/CodeGenCXX/mangle-class-nttp.cpp
@@ -15,7 +15,7 @@
 
 int n = 0;
 // CHECK: define weak_odr void @_Z1fIXtl1BadL_Z1nvv(
-// MSABI: define {{.*}} @"??$f@$2UB@@PEBH1?n@@3HAH0AYAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UB@@PEBHE?n@@3HAH0AYAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1BLPKi0ELi1vv(
 // MSABI: define {{.*}} @"??$f@$2UB@@PEBH0A@H00@@@YAXXZ"
@@ -36,15 +36,19 @@
 
 // Pointers to subobjects.
 struct Nested { union { int k; int arr[2]; }; } nested[2];
-struct Derived : A, Nested { int z; } extern derived;
+struct Derived : A, Nested { int z; A a_field; } extern derived;
 // CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z7derivedE16vv
 // MSABI: define {{.*}} void @"??$f@$2UB@@PEBH56E?derived@@3UDerived@@Az@@@H0AYAXXZ"
 template void f();
-// FIXME: We don't know the MS ABI mangling for array subscripting and
-// past-the-end pointers yet.
-#ifndef _WIN32
+// CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z7derivedE20vv
+// MSABI: define {{.*}} void @"??$f@$2UB@@PEBH566E?derived@@3UDerived@@Aa_field@@a@@@H0AYAXXZ"
+template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z6nestedE_vv
+// MSABI: define {{.*}} void @"??$f@$2UB@@PEBH56CE?nested@@3PAUNested@@A0A@@k@@@H0AYAXXZ"
 template void f();
+// Mangling of pointers to nested array elements and past-the-end pointers
+// is still incorrect in MSVC.
+#ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z6nestedE16_0pvv
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z7derivedE8pvv
@@ -59,14 +63,16 @@
 // CHECK: define weak_odr void @_Z1fIXtl2BRsoKiL_Z7derivedE16vv
 // MSABI: define {{.*}} void @"??$f@$2UBR@@AEBH6E?derived@@3UDerived@@Az@YAXXZ"
 template void f();
-// FIXME: We don't know the MS ABI mangling for array subscripting yet.
-#ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl2BRsoKiL_Z6nestedE_vv
+// MSABI: define {{.*}} void @"??$f@$2UBR@@AEBH6CE?nested@@3PAUNested@@A0A@@k@YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl2BRsoKiL_Z6nestedE12_0vv
+// MSABI: define {{.*}} void @"??$f@$2UBR@@AEBHC6CE?nested@@3PAUNested@@A00@arr@@00YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl2BRsoKiL_Z7derivedE4vv
+// MSABI: define {{.*}} void @"??$f@$2UBR@@AEBH66E?derived@@3UDerived@@AA@@b@YAXXZ"
 template void f();
+#ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl2BRdecvPKiplcvPcadL_Z7derivedELl16vv
 template void f();
 #endif
@@ -77,42 +83,93 @@
 // CHECK: define weak_odr void @_Z1fIXtl1CadsoKiL_Z7derivedE16vv
 // MSABI: define {{.*}} void @"??$f@$2UC@@PEBH56E?derived@@3UDerived@@Az@@YAXXZ"
 template void f();
-#ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl1CadsoKiL_Z7derivedE4vv
+// MSABI: define {{.*}} void @"??$f@$2UC@@PEBH566E?derived@@3UDerived@@AA@@b@@YAXXZ"
 template void f();
-#endif
 
 // Pointers to members.
 struct D { const int Derived::*p; int k; };
 template void f() {}
 // CHECK: define weak_odr void @_Z1fIXtl1DLM7DerivedKi0ELi1vv
-// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H0?0H00@@@YAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@HNH00@@@YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1DEEEvv
-// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H0?0H0AYAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@

[PATCH] D147411: [clang-tidy] Fix readability-static-accessed-through-instance check for anonymous structs

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp:78
+  // Ignore anonymous structs/classes
+  if (StringRef(BaseTypeName).contains("(unnamed "))
+return;

AMS21 wrote:
> I wonder if there's a cleaner and more performant way to check this.
> I've tried `isAnonymousStructOrUnion()` from the `RecordDecl` but that seems 
> to be about something else.
Indeed this seems brittle. Since `RecordDecl` inherits from `NamedDecl`, have 
you tried the `getName()` function? Hopefully it's an empty string and we can 
use that instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147411

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

[PATCH] D147411: [clang-tidy] Fix readability-static-accessed-through-instance check for anonymous structs

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp:78
+  // Ignore anonymous structs/classes
+  if (StringRef(BaseTypeName).contains("(unnamed "))
+return;

carlosgalvezp wrote:
> AMS21 wrote:
> > I wonder if there's a cleaner and more performant way to check this.
> > I've tried `isAnonymousStructOrUnion()` from the `RecordDecl` but that 
> > seems to be about something else.
> Indeed this seems brittle. Since `RecordDecl` inherits from `NamedDecl`, have 
> you tried the `getName()` function? Hopefully it's an empty string and we can 
> use that instead?
Actually this in `NamedDecl` sounds like what you could use here?

```
  /// Get the identifier that names this declaration, if there is one.
  ///
  /// This will return NULL if this declaration has no name (e.g., for
  /// an unnamed class) or if the name is a special name (C++ constructor,
  /// Objective-C selector, etc.).
  IdentifierInfo *getIdentifier() const { return Name.getAsIdentifierInfo(); }
``


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147411

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

[PATCH] D147376: [clang-tidy] Small refactor for ExceptionAnalyzer

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL 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/D147376/new/

https://reviews.llvm.org/D147376

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

[PATCH] D147194: [clang-tidy] fix concat-nest-namespace fix hint remove the macro

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:69
+  }
+  SourceRange{ND->getBeginLoc(), Tok->getEndLoc()}.dump(SM);
+  return SourceRange{ND->getBeginLoc(), Tok->getEndLoc()};

Remove this debug.



Comment at: 
clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:88
+  StringRef TokText = getRawStringRef(TokRange, SM, LangOpts);
+  if (TokText != "// namespace " + ND->getNameAsString())
+return DefaultSourceRange;

what if it is //namespace ? or /* namespace XYZ */



Comment at: 
clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:125
+
+  for (size_t Index = 0; Index < Namespaces.size(); Index++) {
+if (Namespaces[Index]->isNested()) {

remove redundant {} from this function, they not needed for single line 
statements. 



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp:172
+// CHECK-FIXES: #undef N43_INNER
+
 int main() {

add same test but without spaces and move //CHECK-MESSAGES outside namespace. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147194

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

[PATCH] D146881: [clang-tidy] Fix findNextTokenSkippingComments & rangeContainsExpansionsOrDirectives

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd79715b6a09c: [clang-tidy] Fix findNextTokenSkippingComments 
&… (authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146881

Files:
  clang-tools-extra/clang-tidy/utils/LexerUtils.cpp


Index: clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
@@ -78,11 +78,16 @@
 std::optional
 findNextTokenSkippingComments(SourceLocation Start, const SourceManager &SM,
   const LangOptions &LangOpts) {
-  std::optional CurrentToken;
-  do {
-CurrentToken = Lexer::findNextToken(Start, SM, LangOpts);
-  } while (CurrentToken && CurrentToken->is(tok::comment));
-  return CurrentToken;
+  while (Start.isValid()) {
+std::optional CurrentToken =
+Lexer::findNextToken(Start, SM, LangOpts);
+if (!CurrentToken || !CurrentToken->is(tok::comment))
+  return CurrentToken;
+
+Start = CurrentToken->getLocation();
+  }
+
+  return std::nullopt;
 }
 
 bool rangeContainsExpansionsOrDirectives(SourceRange Range,
@@ -91,7 +96,7 @@
   assert(Range.isValid() && "Invalid Range for relexing provided");
   SourceLocation Loc = Range.getBegin();
 
-  while (Loc < Range.getEnd()) {
+  while (Loc <= Range.getEnd()) {
 if (Loc.isMacroID())
   return true;
 
@@ -103,7 +108,7 @@
 if (Tok->is(tok::hash))
   return true;
 
-Loc = Lexer::getLocForEndOfToken(Loc, 0, SM, LangOpts).getLocWithOffset(1);
+Loc = Tok->getLocation();
   }
 
   return false;


Index: clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
@@ -78,11 +78,16 @@
 std::optional
 findNextTokenSkippingComments(SourceLocation Start, const SourceManager &SM,
   const LangOptions &LangOpts) {
-  std::optional CurrentToken;
-  do {
-CurrentToken = Lexer::findNextToken(Start, SM, LangOpts);
-  } while (CurrentToken && CurrentToken->is(tok::comment));
-  return CurrentToken;
+  while (Start.isValid()) {
+std::optional CurrentToken =
+Lexer::findNextToken(Start, SM, LangOpts);
+if (!CurrentToken || !CurrentToken->is(tok::comment))
+  return CurrentToken;
+
+Start = CurrentToken->getLocation();
+  }
+
+  return std::nullopt;
 }
 
 bool rangeContainsExpansionsOrDirectives(SourceRange Range,
@@ -91,7 +96,7 @@
   assert(Range.isValid() && "Invalid Range for relexing provided");
   SourceLocation Loc = Range.getBegin();
 
-  while (Loc < Range.getEnd()) {
+  while (Loc <= Range.getEnd()) {
 if (Loc.isMacroID())
   return true;
 
@@ -103,7 +108,7 @@
 if (Tok->is(tok::hash))
   return true;
 
-Loc = Lexer::getLocForEndOfToken(Loc, 0, SM, LangOpts).getLocWithOffset(1);
+Loc = Tok->getLocation();
   }
 
   return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] d79715b - [clang-tidy] Fix findNextTokenSkippingComments & rangeContainsExpansionsOrDirectives

[Piotr Zegar via cfe-commits]

Author: Piotr Zegar
Date: 2023-04-02T16:05:15Z
New Revision: d79715b6a09c538ac60a8259d540a94d17fbc824

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

LOG: [clang-tidy] Fix findNextTokenSkippingComments & 
rangeContainsExpansionsOrDirectives

findNextTokenSkippingComments is actually a endless loop,
implementing it correctly.
rangeContainsExpansionsOrDirectives were skiping every second
token, if there were no whitespaces bettwen tokens.

Reviewed By: carlosgalvezp

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/utils/LexerUtils.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp 
b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
index 49718962f62f1..1b690e9f3db67 100644
--- a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
@@ -78,11 +78,16 @@ SourceLocation findNextTerminator(SourceLocation Start, 
const SourceManager &SM,
 std::optional
 findNextTokenSkippingComments(SourceLocation Start, const SourceManager &SM,
   const LangOptions &LangOpts) {
-  std::optional CurrentToken;
-  do {
-CurrentToken = Lexer::findNextToken(Start, SM, LangOpts);
-  } while (CurrentToken && CurrentToken->is(tok::comment));
-  return CurrentToken;
+  while (Start.isValid()) {
+std::optional CurrentToken =
+Lexer::findNextToken(Start, SM, LangOpts);
+if (!CurrentToken || !CurrentToken->is(tok::comment))
+  return CurrentToken;
+
+Start = CurrentToken->getLocation();
+  }
+
+  return std::nullopt;
 }
 
 bool rangeContainsExpansionsOrDirectives(SourceRange Range,
@@ -91,7 +96,7 @@ bool rangeContainsExpansionsOrDirectives(SourceRange Range,
   assert(Range.isValid() && "Invalid Range for relexing provided");
   SourceLocation Loc = Range.getBegin();
 
-  while (Loc < Range.getEnd()) {
+  while (Loc <= Range.getEnd()) {
 if (Loc.isMacroID())
   return true;
 
@@ -103,7 +108,7 @@ bool rangeContainsExpansionsOrDirectives(SourceRange Range,
 if (Tok->is(tok::hash))
   return true;
 
-Loc = Lexer::getLocForEndOfToken(Loc, 0, SM, LangOpts).getLocWithOffset(1);
+Loc = Tok->getLocation();
   }
 
   return false;



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

[PATCH] D147414: [python] Expose clang_Location_isInSystemHeader

[Artur Ryt via Phabricator via cfe-commits]

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

Add `is_in_system_header` property for `Location` class.

Corresponding unit test was also added.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147414

Files:
  clang/bindings/python/clang/cindex.py
  clang/bindings/python/tests/cindex/test_location.py


Index: clang/bindings/python/tests/cindex/test_location.py
===
--- clang/bindings/python/tests/cindex/test_location.py
+++ clang/bindings/python/tests/cindex/test_location.py
@@ -7,6 +7,7 @@
 from clang.cindex import File
 from clang.cindex import SourceLocation
 from clang.cindex import SourceRange
+from clang.cindex import TranslationUnit
 from .util import get_cursor
 from .util import get_tu
 
@@ -103,3 +104,17 @@
 location3 = SourceLocation.from_position(tu, file, 1, 6)
 range3 = SourceRange.from_locations(location1, location3)
 self.assertNotEqual(range1, range3)
+
+def test_is_system_location(self):
+header = os.path.normpath('./fake/fake.h')
+tu = TranslationUnit.from_source('fake.c', 
[f'-isystem{os.path.dirname(header)}'], unsaved_files = [
+('fake.c', """
+#include 
+int one;
+"""),
+(header, "int two();")
+])
+one = get_cursor(tu, 'one')
+two = get_cursor(tu, 'two')
+self.assertFalse(one.location.is_in_system_header)
+self.assertTrue(two.location.is_in_system_header)
Index: clang/bindings/python/clang/cindex.py
===
--- clang/bindings/python/clang/cindex.py
+++ clang/bindings/python/clang/cindex.py
@@ -286,6 +286,11 @@
 """Get the file offset represented by this source location."""
 return self._get_instantiation()[3]
 
+@property
+def is_in_system_header(self):
+"""Returns true if the given source location is in a system header."""
+return conf.lib.clang_Location_isInSystemHeader(self)
+
 def __eq__(self, other):
 return conf.lib.clang_equalLocations(self, other)
 
@@ -4131,6 +4136,10 @@
[Cursor],
c_longlong),
 
+  ("clang_Location_isInSystemHeader",
+   [SourceLocation],
+   bool),
+
   ("clang_Type_getAlignOf",
[Type],
c_longlong),


Index: clang/bindings/python/tests/cindex/test_location.py
===
--- clang/bindings/python/tests/cindex/test_location.py
+++ clang/bindings/python/tests/cindex/test_location.py
@@ -7,6 +7,7 @@
 from clang.cindex import File
 from clang.cindex import SourceLocation
 from clang.cindex import SourceRange
+from clang.cindex import TranslationUnit
 from .util import get_cursor
 from .util import get_tu
 
@@ -103,3 +104,17 @@
 location3 = SourceLocation.from_position(tu, file, 1, 6)
 range3 = SourceRange.from_locations(location1, location3)
 self.assertNotEqual(range1, range3)
+
+def test_is_system_location(self):
+header = os.path.normpath('./fake/fake.h')
+tu = TranslationUnit.from_source('fake.c', [f'-isystem{os.path.dirname(header)}'], unsaved_files = [
+('fake.c', """
+#include 
+int one;
+"""),
+(header, "int two();")
+])
+one = get_cursor(tu, 'one')
+two = get_cursor(tu, 'two')
+self.assertFalse(one.location.is_in_system_header)
+self.assertTrue(two.location.is_in_system_header)
Index: clang/bindings/python/clang/cindex.py
===
--- clang/bindings/python/clang/cindex.py
+++ clang/bindings/python/clang/cindex.py
@@ -286,6 +286,11 @@
 """Get the file offset represented by this source location."""
 return self._get_instantiation()[3]
 
+@property
+def is_in_system_header(self):
+"""Returns true if the given source location is in a system header."""
+return conf.lib.clang_Location_isInSystemHeader(self)
+
 def __eq__(self, other):
 return conf.lib.clang_equalLocations(self, other)
 
@@ -4131,6 +4136,10 @@
[Cursor],
c_longlong),
 
+  ("clang_Location_isInSystemHeader",
+   [SourceLocation],
+   bool),
+
   ("clang_Type_getAlignOf",
[Type],
c_longlong),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147414: [python] Expose clang_Location_isInSystemHeader

[Artur Ryt via Phabricator via cfe-commits]

R2RT added a comment.

I thought about adding it into release notes, but recently Python section has 
got removed from ReleaseNotes.rst, so I am not sure what to do now: 
https://reviews.llvm.org/D142578


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147414

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

[PATCH] D147417: [clang-tidy] Do not emit bugprone-exception-escape warnings from coroutines

[Deniz Evrenci via Phabricator via cfe-commits]

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

All exceptions thrown in coroutine bodies are caught and
unhandled_exception member of the coroutine promise type is called.
In accordance with the existing rules of diagnostics related to
exceptions thrown in functions marked noexcept, even if the promise
type's constructor, get_return_object, or unhandled_exception
throws, diagnostics should not be emitted.

Fixes #61905.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147417

Files:
  clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
@@ -0,0 +1,121 @@
+// RUN: %check_clang_tidy -std=c++20 %s bugprone-exception-escape %t -- \
+// RUN: -- -fexceptions
+
+namespace std {
+
+template  struct coroutine_traits {
+  using promise_type = typename Ret::promise_type;
+};
+
+template  struct coroutine_handle {
+  static coroutine_handle from_address(void *) noexcept;
+  static coroutine_handle from_promise(Promise &promise);
+  constexpr void *address() const noexcept;
+};
+
+template <> struct coroutine_handle {
+  template 
+  coroutine_handle(coroutine_handle) noexcept;
+  static coroutine_handle from_address(void *);
+  constexpr void *address() const noexcept;
+};
+
+struct suspend_always {
+  bool await_ready() noexcept { return false; }
+  void await_suspend(coroutine_handle<>) noexcept {}
+  void await_resume() noexcept {}
+};
+
+struct suspend_never {
+  bool await_ready() noexcept { return true; }
+  void await_suspend(coroutine_handle<>) noexcept {}
+  void await_resume() noexcept {}
+};
+
+} // namespace std
+
+template  struct promise;
+
+template  struct task {
+  using promise_type = promise;
+
+  explicit task(promise_type &p) {
+throw 1;
+p.return_val = this;
+  }
+
+  T value;
+};
+
+template  struct promise {
+  task get_return_object() { return task{*this}; }
+
+  std::suspend_never initial_suspend() const noexcept { return {}; }
+
+  std::suspend_never final_suspend() const noexcept { return {}; }
+
+  template  void return_value(U &&val) {
+return_val->value = static_cast(val);
+  }
+
+  void unhandled_exception() { throw 1; }
+
+  task *return_val;
+};
+
+task a_ShouldNotDiag(const int a, const int b) {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:11: warning: an exception may be thrown in
+  // function 'a_ShouldNotDiag' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  co_return a / b;
+}
+
+task b_ShouldNotDiag(const int a, const int b) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:11: warning: an exception may be thrown in
+  // function 'b_ShouldNotDiag' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  co_return a / b;
+}
+
+int c_ShouldDiag(const int a, const int b) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in
+  // function 'c_ShouldDiag' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  return a / b;
+}
+
+const auto d_ShouldNotDiag = [](const int a, const int b) -> task {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:30: warning: an exception may be thrown in
+  // function 'operator()' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  co_return a / b;
+};
+
+const auto e_ShouldNotDiag = [](const int a,
+const int b) noexcept -> task {
+  // CHECK-MESSAGES-NOT: :[[@LINE-2]]:30: warning: an exception may be thrown in
+  // function 'operator()' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  co_return a / b;
+};
+
+const auto f_ShouldDiag = [](const int a, const int b) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-2]]:27: warning: an exception may be thrown in
+  // function 'operator()' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  return a / b;
+};
+
+// CHECK-MESSAGES
Index: clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -51,12 +51,15 @@
 }
 
 void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) {
+  auto IsCoroMatcher =
+  hasDescendant(expr(anyOf(coyieldExpr(), coreturnStmt(), coawaitExpr(;
   Finder->addMatcher(
   functionDecl(anyOf(isNoThrow(), cxxDestructorDecl(),
  cxxConstructorDecl(isMoveConstructor()),
 

[PATCH] D147419: [clang-tidy] ignore NRVO const variables in performance-no-automatic-move.

[Logan Gnanapragasam via Phabricator via cfe-commits]

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

In the following code:

  cc
  struct Obj {
Obj();
Obj(const Obj &);
Obj(Obj &&);
virtual ~Obj();
  };
  
  Obj ConstNrvo() {
const Obj obj;
return obj;
  }

performance-no-automatic-move warns about the constness of `obj`. However, NRVO
is applied to `obj`, so the `const` should have no effect on performance.

This change modifies the matcher to exclude NRVO variables.

#clang-tidy 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147419

Files:
  clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
@@ -33,10 +33,15 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents 
automatic move [performance-no-automatic-move]
 }
 
-Obj PositiveSelfConstValue() {
-  const Obj obj = Make();
-  return obj;
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents 
automatic move [performance-no-automatic-move]
+Obj PositiveCantNrvo(bool b) {
+  const Obj obj1;
+  const Obj obj2;
+  if (b) {
+return obj1;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: constness of 'obj1' prevents 
automatic move [performance-no-automatic-move]
+  }
+  return obj2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj1' prevents 
automatic move [performance-no-automatic-move]
 }
 
 // FIXME: Ideally we would warn here too.
@@ -54,18 +59,32 @@
 // Negatives.
 
 StatusOr Temporary() {
-  return Make();
+  return Make();
 }
 
 StatusOr ConstTemporary() {
   return Make();
 }
 
-StatusOr Nrvo() {
+StatusOr ConvertingMoveConstructor() {
   Obj obj = Make();
   return obj;
 }
 
+Obj ConstNrvo() {
+  const Obj obj = Make();
+  return obj;
+}
+
+Obj NotNrvo(bool b) {
+  Obj obj1;
+  Obj obj2;
+  if (b) {
+return obj1;
+  }
+  return obj2;
+}
+
 StatusOr Ref() {
   Obj &obj = Make();
   return obj;
Index: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
@@ -16,6 +16,12 @@
 
 namespace clang::tidy::performance {
 
+namespace {
+
+AST_MATCHER(VarDecl, isNRVOVariable) { return Node.isNRVOVariable(); }
+
+} // namespace
+
 NoAutomaticMoveCheck::NoAutomaticMoveCheck(StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -23,8 +29,9 @@
   utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
 
 void NoAutomaticMoveCheck::registerMatchers(MatchFinder *Finder) {
-  const auto ConstLocalVariable =
+  const auto NonNrvoConstLocalVariable =
   varDecl(hasLocalStorage(), unless(hasType(lValueReferenceType())),
+  unless(isNRVOVariable()),
   hasType(qualType(
   isConstQualified(),
   hasCanonicalType(matchers::isExpensiveToCopy()),
@@ -48,7 +55,7 @@
cxxConstructExpr(
hasDeclaration(LValueRefCtor),
hasArgument(0, ignoringParenImpCasts(declRefExpr(
-  to(ConstLocalVariable)
+  to(NonNrvoConstLocalVariable)
.bind("ctor_call")),
   this);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
@@ -33,10 +33,15 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
 }
 
-Obj PositiveSelfConstValue() {
-  const Obj obj = Make();
-  return obj;
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+Obj PositiveCantNrvo(bool b) {
+  const Obj obj1;
+  const Obj obj2;
+  if (b) {
+return obj1;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: constness of 'obj1' prevents automatic move [performance-no-automatic-mo

[PATCH] D147419: [clang-tidy] ignore NRVO const variables in performance-no-automatic-move.

[Logan Gnanapragasam via Phabricator via cfe-commits]

gnanabit updated this revision to Diff 510374.
gnanabit added a comment.

Fix typo in `CHECK-MESSAGES` (should warn about `obj2`, not `obj1`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147419

Files:
  clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
@@ -33,10 +33,15 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents 
automatic move [performance-no-automatic-move]
 }
 
-Obj PositiveSelfConstValue() {
-  const Obj obj = Make();
-  return obj;
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents 
automatic move [performance-no-automatic-move]
+Obj PositiveCantNrvo(bool b) {
+  const Obj obj1;
+  const Obj obj2;
+  if (b) {
+return obj1;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: constness of 'obj1' prevents 
automatic move [performance-no-automatic-move]
+  }
+  return obj2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj2' prevents 
automatic move [performance-no-automatic-move]
 }
 
 // FIXME: Ideally we would warn here too.
@@ -54,18 +59,32 @@
 // Negatives.
 
 StatusOr Temporary() {
-  return Make();
+  return Make();
 }
 
 StatusOr ConstTemporary() {
   return Make();
 }
 
-StatusOr Nrvo() {
+StatusOr ConvertingMoveConstructor() {
   Obj obj = Make();
   return obj;
 }
 
+Obj ConstNrvo() {
+  const Obj obj = Make();
+  return obj;
+}
+
+Obj NotNrvo(bool b) {
+  Obj obj1;
+  Obj obj2;
+  if (b) {
+return obj1;
+  }
+  return obj2;
+}
+
 StatusOr Ref() {
   Obj &obj = Make();
   return obj;
Index: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
@@ -16,6 +16,12 @@
 
 namespace clang::tidy::performance {
 
+namespace {
+
+AST_MATCHER(VarDecl, isNRVOVariable) { return Node.isNRVOVariable(); }
+
+} // namespace
+
 NoAutomaticMoveCheck::NoAutomaticMoveCheck(StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -23,8 +29,9 @@
   utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
 
 void NoAutomaticMoveCheck::registerMatchers(MatchFinder *Finder) {
-  const auto ConstLocalVariable =
+  const auto NonNrvoConstLocalVariable =
   varDecl(hasLocalStorage(), unless(hasType(lValueReferenceType())),
+  unless(isNRVOVariable()),
   hasType(qualType(
   isConstQualified(),
   hasCanonicalType(matchers::isExpensiveToCopy()),
@@ -48,7 +55,7 @@
cxxConstructExpr(
hasDeclaration(LValueRefCtor),
hasArgument(0, ignoringParenImpCasts(declRefExpr(
-  to(ConstLocalVariable)
+  to(NonNrvoConstLocalVariable)
.bind("ctor_call")),
   this);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
@@ -33,10 +33,15 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
 }
 
-Obj PositiveSelfConstValue() {
-  const Obj obj = Make();
-  return obj;
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+Obj PositiveCantNrvo(bool b) {
+  const Obj obj1;
+  const Obj obj2;
+  if (b) {
+return obj1;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: constness of 'obj1' prevents automatic move [performance-no-automatic-move]
+  }
+  return obj2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj2' prevents automatic move [performance-no-automatic-move]
 }
 
 // FIXME: Ideally we would warn here too.
@@ -54,18 +59,32 @@
 // Negatives.
 
 StatusOr Temporary() {
-  return Make();
+  return Make();
 }
 
 StatusOr ConstTemporary() {
   return Make();
 }
 
-StatusOr Nrvo() {
+StatusOr ConvertingMoveConstructor() {
   Obj obj = Make();
   return obj;
 }
 
+Obj ConstNrvo() {
+  const Obj obj = Make();
+  return obj;
+}
+
+Obj NotNrvo(bool b) {
+  Obj obj1;
+  Obj obj2;
+  if 

[PATCH] D147417: [clang-tidy] Do not emit bugprone-exception-escape warnings from coroutines

[Deniz Evrenci via Phabricator via cfe-commits]

denizevrenci updated this revision to Diff 510378.
denizevrenci added a comment.

Remove line breaks in CHECK-MESSAGES lines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147417

Files:
  clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
@@ -0,0 +1,115 @@
+// RUN: %check_clang_tidy -std=c++20 %s bugprone-exception-escape %t -- \
+// RUN: -- -fexceptions
+
+namespace std {
+
+template  struct coroutine_traits {
+  using promise_type = typename Ret::promise_type;
+};
+
+template  struct coroutine_handle {
+  static coroutine_handle from_address(void *) noexcept;
+  static coroutine_handle from_promise(Promise &promise);
+  constexpr void *address() const noexcept;
+};
+
+template <> struct coroutine_handle {
+  template 
+  coroutine_handle(coroutine_handle) noexcept;
+  static coroutine_handle from_address(void *);
+  constexpr void *address() const noexcept;
+};
+
+struct suspend_always {
+  bool await_ready() noexcept { return false; }
+  void await_suspend(coroutine_handle<>) noexcept {}
+  void await_resume() noexcept {}
+};
+
+struct suspend_never {
+  bool await_ready() noexcept { return true; }
+  void await_suspend(coroutine_handle<>) noexcept {}
+  void await_resume() noexcept {}
+};
+
+} // namespace std
+
+template  struct promise;
+
+template  struct task {
+  using promise_type = promise;
+
+  explicit task(promise_type &p) {
+throw 1;
+p.return_val = this;
+  }
+
+  T value;
+};
+
+template  struct promise {
+  task get_return_object() { return task{*this}; }
+
+  std::suspend_never initial_suspend() const noexcept { return {}; }
+
+  std::suspend_never final_suspend() const noexcept { return {}; }
+
+  template  void return_value(U &&val) {
+return_val->value = static_cast(val);
+  }
+
+  void unhandled_exception() { throw 1; }
+
+  task *return_val;
+};
+
+task a_ShouldNotDiag(const int a, const int b) {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:11: warning: an exception may be thrown in function 'a_ShouldNotDiag' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  co_return a / b;
+}
+
+task b_ShouldNotDiag(const int a, const int b) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:11: warning: an exception may be thrown in function 'b_ShouldNotDiag' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  co_return a / b;
+}
+
+int c_ShouldDiag(const int a, const int b) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'c_ShouldDiag' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  return a / b;
+}
+
+const auto d_ShouldNotDiag = [](const int a, const int b) -> task {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:30: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  co_return a / b;
+};
+
+const auto e_ShouldNotDiag = [](const int a,
+const int b) noexcept -> task {
+  // CHECK-MESSAGES-NOT: :[[@LINE-2]]:30: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  co_return a / b;
+};
+
+const auto f_ShouldDiag = [](const int a, const int b) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-2]]:27: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  return a / b;
+};
+
+// CHECK-MESSAGES
Index: clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -51,12 +51,15 @@
 }
 
 void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) {
+  auto IsCoroMatcher =
+  hasDescendant(expr(anyOf(coyieldExpr(), coreturnStmt(), coawaitExpr(;
   Finder->addMatcher(
   functionDecl(anyOf(isNoThrow(), cxxDestructorDecl(),
  cxxConstructorDecl(isMoveConstructor()),
  cxxMethodDecl(isMoveAssignmentOperator()),
  hasName("main"), hasName("swap"),
- isEnabled(FunctionsThatShouldNotThrow)))
+ isEnabled(FunctionsThatShouldNotThrow)),
+   unless(IsCoroMatcher))
   .bind("thrower"),
   this);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147417: [clang-tidy] Do not emit bugprone-exception-escape warnings from coroutines

[Deniz Evrenci via Phabricator via cfe-commits]

denizevrenci updated this revision to Diff 510383.
denizevrenci added a comment.

Fix the line number for the warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147417

Files:
  clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
@@ -0,0 +1,115 @@
+// RUN: %check_clang_tidy -std=c++20 %s bugprone-exception-escape %t -- \
+// RUN: -- -fexceptions
+
+namespace std {
+
+template  struct coroutine_traits {
+  using promise_type = typename Ret::promise_type;
+};
+
+template  struct coroutine_handle {
+  static coroutine_handle from_address(void *) noexcept;
+  static coroutine_handle from_promise(Promise &promise);
+  constexpr void *address() const noexcept;
+};
+
+template <> struct coroutine_handle {
+  template 
+  coroutine_handle(coroutine_handle) noexcept;
+  static coroutine_handle from_address(void *);
+  constexpr void *address() const noexcept;
+};
+
+struct suspend_always {
+  bool await_ready() noexcept { return false; }
+  void await_suspend(coroutine_handle<>) noexcept {}
+  void await_resume() noexcept {}
+};
+
+struct suspend_never {
+  bool await_ready() noexcept { return true; }
+  void await_suspend(coroutine_handle<>) noexcept {}
+  void await_resume() noexcept {}
+};
+
+} // namespace std
+
+template  struct promise;
+
+template  struct task {
+  using promise_type = promise;
+
+  explicit task(promise_type &p) {
+throw 1;
+p.return_val = this;
+  }
+
+  T value;
+};
+
+template  struct promise {
+  task get_return_object() { return task{*this}; }
+
+  std::suspend_never initial_suspend() const noexcept { return {}; }
+
+  std::suspend_never final_suspend() const noexcept { return {}; }
+
+  template  void return_value(U &&val) {
+return_val->value = static_cast(val);
+  }
+
+  void unhandled_exception() { throw 1; }
+
+  task *return_val;
+};
+
+task a_ShouldNotDiag(const int a, const int b) {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:11: warning: an exception may be thrown in function 'a_ShouldNotDiag' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  co_return a / b;
+}
+
+task b_ShouldNotDiag(const int a, const int b) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:11: warning: an exception may be thrown in function 'b_ShouldNotDiag' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  co_return a / b;
+}
+
+int c_ShouldDiag(const int a, const int b) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'c_ShouldDiag' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  return a / b;
+}
+
+const auto d_ShouldNotDiag = [](const int a, const int b) -> task {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:30: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  co_return a / b;
+};
+
+const auto e_ShouldNotDiag = [](const int a,
+const int b) noexcept -> task {
+  // CHECK-MESSAGES-NOT: :[[@LINE-2]]:30: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  co_return a / b;
+};
+
+const auto f_ShouldDiag = [](const int a, const int b) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
+  if (b == 0)
+throw b;
+
+  return a / b;
+};
+
+// CHECK-MESSAGES
Index: clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -51,12 +51,15 @@
 }
 
 void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) {
+  auto IsCoroMatcher =
+  hasDescendant(expr(anyOf(coyieldExpr(), coreturnStmt(), coawaitExpr(;
   Finder->addMatcher(
   functionDecl(anyOf(isNoThrow(), cxxDestructorDecl(),
  cxxConstructorDecl(isMoveConstructor()),
  cxxMethodDecl(isMoveAssignmentOperator()),
  hasName("main"), hasName("swap"),
- isEnabled(FunctionsThatShouldNotThrow)))
+ isEnabled(FunctionsThatShouldNotThrow)),
+   unless(IsCoroMatcher))
   .bind("thrower"),
   this);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147417: [clang-tidy] Do not emit bugprone-exception-escape warnings from coroutines

[Deniz Evrenci via Phabricator via cfe-commits]

denizevrenci added a comment.

I don't have commit access so after the review is complete, please commit this 
diff in my place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147417

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

[PATCH] D147327: [clang-format] Add option for having one port per line in Verilog

[sstwcw via Phabricator via cfe-commits]

sstwcw updated this revision to Diff 510385.
sstwcw added a comment.

- Use lambda


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147327

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/ConfigParseTest.cpp
  clang/unittests/Format/FormatTestVerilog.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1579,6 +1579,18 @@
   EXPECT_TOKEN_PRECEDENCE(Tokens[1], prec::Assignment);
   EXPECT_TOKEN(Tokens[3], tok::lessequal, TT_BinaryOperator);
   EXPECT_TOKEN_PRECEDENCE(Tokens[3], prec::Relational);
+
+  // Port lists in module instantiation.
+  Tokens = Annotate("module_x instance_1(port_1), instance_2(port_2);");
+  ASSERT_EQ(Tokens.size(), 12u);
+  EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_VerilogInstancePortLParen);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_VerilogInstancePortLParen);
+  Tokens = Annotate("module_x #(parameter) instance_1(port_1), "
+"instance_2(port_2);");
+  ASSERT_EQ(Tokens.size(), 16u);
+  EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_VerilogInstancePortLParen);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_VerilogInstancePortLParen);
+  EXPECT_TOKEN(Tokens[11], tok::l_paren, TT_VerilogInstancePortLParen);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandConstructors) {
Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- clang/unittests/Format/FormatTestVerilog.cpp
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -673,6 +673,77 @@
"  x = x;");
 }
 
+TEST_F(FormatTestVerilog, Instantiation) {
+  // Without ports.
+  verifyFormat("ffnand ff1;");
+  // With named ports.
+  verifyFormat("ffnand ff1(.qbar(out1),\n"
+   "   .clear(in1),\n"
+   "   .preset(in2));");
+  // With wildcard.
+  verifyFormat("ffnand ff1(.qbar(out1),\n"
+   "   .clear(in1),\n"
+   "   .preset(in2),\n"
+   "   .*);");
+  verifyFormat("ffnand ff1(.*,\n"
+   "   .qbar(out1),\n"
+   "   .clear(in1),\n"
+   "   .preset(in2));");
+  // With unconnected ports.
+  verifyFormat("ffnand ff1(.q(),\n"
+   "   .qbar(out1),\n"
+   "   .clear(in1),\n"
+   "   .preset(in2));");
+  verifyFormat("ffnand ff1(.q(),\n"
+   "   .qbar(),\n"
+   "   .clear(),\n"
+   "   .preset());");
+  verifyFormat("ffnand ff1(,\n"
+   "   .qbar(out1),\n"
+   "   .clear(in1),\n"
+   "   .preset(in2));");
+  // With positional ports.
+  verifyFormat("ffnand ff1(out1,\n"
+   "   in1,\n"
+   "   in2);");
+  verifyFormat("ffnand ff1(,\n"
+   "   out1,\n"
+   "   in1,\n"
+   "   in2);");
+  // Multiple instantiations.
+  verifyFormat("ffnand ff1(.q(),\n"
+   "   .qbar(out1),\n"
+   "   .clear(in1),\n"
+   "   .preset(in2)),\n"
+   "   ff1(.q(),\n"
+   "   .qbar(out1),\n"
+   "   .clear(in1),\n"
+   "   .preset(in2));");
+  verifyFormat("ffnand //\n"
+   "ff1(.q(),\n"
+   ".qbar(out1),\n"
+   ".clear(in1),\n"
+   ".preset(in2)),\n"
+   "ff1(.q(),\n"
+   ".qbar(out1),\n"
+   ".clear(in1),\n"
+   ".preset(in2));");
+  // With breaking between instance ports disabled.
+  auto Style = getDefaultStyle();
+  Style.VerilogBreakBetweenInstancePorts = false;
+  verifyFormat("ffnand ff1;", Style);
+  verifyFormat("ffnand ff1(.qbar(out1), .clear(in1), .preset(in2), .*);",
+   Style);
+  verifyFormat("ffnand ff1(out1, in1, in2);", Style);
+  verifyFormat("ffnand ff1(.q(), .qbar(out1), .clear(in1), .preset(in2)),\n"
+   "   ff1(.q(), .qbar(out1), .clear(in1), .preset(in2));",
+   Style);
+  verifyFormat("ffnand //\n"
+   "ff1(.q(), .qbar(out1), .clear(in1), .preset(in2)),\n"
+   "ff1(.q(), .qbar(out1), .clear(in1), .preset(in2));",
+   Style);
+}
+
 TEST_F(FormatTestVerilog, Operators) {
   // Test that unary operators are not followed by space.
   verifyFormat("x = +x;");
Index: clang/unittests/Format/ConfigParseTe

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

[Iain Sandoe via Phabricator via cfe-commits]

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

rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145965

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Lookup.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaLookup.cpp
  clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
  clang/test/CXX/module/basic/basic.def.odr/p4.cppm
  clang/test/CXX/module/basic/basic.link/p2.cppm
  clang/test/CXX/module/module.import/p2.cpp
  clang/test/CXX/module/module.interface/p2.cpp
  clang/test/CXX/module/module.interface/p7.cpp
  clang/test/CXX/module/module.reach/ex1.cpp
  clang/test/CXX/module/module.reach/p2.cpp
  clang/test/CXX/module/module.reach/p5.cpp
  clang/test/Modules/Reachability-template-default-arg.cpp
  clang/test/Modules/cxx20-10-1-ex2.cpp
  clang/test/Modules/deduction-guide3.cppm
  clang/test/Modules/diagnose-missing-import.m

Index: clang/test/Modules/diagnose-missing-import.m
===
--- clang/test/Modules/diagnose-missing-import.m
+++ clang/test/Modules/diagnose-missing-import.m
@@ -6,9 +6,9 @@
 
 void foo(void) {
   XYZLogEvent(xyzRiskyCloseOpenParam, xyzRiskyCloseOpenParam); // expected-error {{call to undeclared function 'XYZLogEvent'; ISO C99 and later do not support implicit function declarations}} \
-  expected-error {{declaration of 'XYZLogEvent' must be imported}} \
-  expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}} \
-  expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}}
+  expected-error {{declaration of 'XYZLogEvent' is internal to 'NCI.A'}} \
+  expected-error {{declaration of 'xyzRiskyCloseOpenParam' is internal to 'NCI.A'}} \
+  expected-error {{declaration of 'xyzRiskyCloseOpenParam' is internal to 'NCI.A'}}
 }
 
 // expected-note@Inputs/diagnose-missing-import/a.h:5 {{declaration here is not visible}}
Index: clang/test/Modules/deduction-guide3.cppm
===
--- clang/test/Modules/deduction-guide3.cppm
+++ clang/test/Modules/deduction-guide3.cppm
@@ -19,8 +19,8 @@
 //--- Use.cpp
 import Templ;
 void func() {
-Templ t(5); // expected-error {{declaration of 'Templ' must be imported from module 'Templ' before it is required}}
+Templ t(5); // expected-error {{declaration of 'Templ' is private to module 'Templ'}}
 // expected-error@-1 {{unknown type name 'Templ'}}
-// expected-n...@templ.cppm:3 {{declaration here is not visible}}
+// expected-n...@templ.cppm:3 {{export the declaration to make it available}}
 }
 
Index: clang/test/Modules/cxx20-10-1-ex2.cpp
===
--- clang/test/Modules/cxx20-10-1-ex2.cpp
+++ clang/test/Modules/cxx20-10-1-ex2.cpp
@@ -53,8 +53,8 @@
 //--- std10-1-ex2-tu6.cpp
 import B;
 // error, n is module-local and this is not a module.
-int &c = n; // expected-error {{declaration of 'n' must be imported}}
-// expected-note@* {{declaration here is not visible}}
+int &c = n; // expected-error {{declaration of 'n' is private to module 'B'}}
+// expected-note@* {{export the declaration to make it available}}
 
 //--- std10-1-ex2-tu7.cpp
 // expected-no-diagnostics
Index: clang/test/Modules/Reachability-template-default-arg.cpp
===
--- clang/test/Modules/Reachability-template-default-arg.cpp
+++ clang/test/Modules/Reachability-template-default-arg.cpp
@@ -18,6 +18,6 @@
 import template_default_arg;
 void bar() {
   A<> a0;
-  A a1; // expected-error {{declaration of 't' must be imported from module 'template_default_arg' before it is required}}
-   // expected-note@* {{declaration here is not visible}}
+  A a1; // expected-error {{declaration of 't' is private to module 'template_default_arg'}}
+   // expected-note@* {{export the declaration to make it available}}
 }
Index: clang/test/CXX/module/module.reach/p5.cpp
===
--- clang/test/CXX/module/module.reach/p5.cpp
+++ clang/test/CXX/module/module.reach/p5.cpp
@@ -14,5 +14,5 @@
 export module B;
 import A;
 Y y; // OK, definition of X is reachable
-X x; // expected-error {{declaration of 'X' must be imported from module 'A' before it is required}}
- // expected-note@* {{declaration here is not visib

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

[Iain Sandoe via Phabricator via cfe-commits]

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

I updated this because I am going to push the latest version of the `P1815` 
patch and that depends on the lookup changes.




Comment at: clang/lib/Sema/SemaLookup.cpp:2082
+Module *DeclModule = SemaRef.getOwningModule(D);
+if (DeclModule && !DeclModule->isModuleMapModule() &&
+!SemaRef.isModuleUnitOfCurrentTU(DeclModule) &&

ChuanqiXu wrote:
> We should check header units here.
The point of checking module map modules was to avoid affecting clang modules 
with the change in semantics.

At the moment, I am not sure why we could exclude lookup from finding decls in 
an imported header unit?




Comment at: clang/lib/Sema/SemaLookup.cpp:2101
+  if (isVisible(SemaRef, ND)) {
+if (SemaRef.getLangOpts().CPlusPlusModules && ND->isFromASTFile()) {
+  // The module that owns the decl is visible; However

ChuanqiXu wrote:
> Let's not use `isFromASTFile()`. It is a low level API without higher level 
> semantics. I think it is good enough to check the module of ND.
lookup is very heavily used; the purpose of the isFromAST() check is to 
short-circuit the more expensive checks when we know that a decl must be in the 
same TU (i.e. it is not from an AST file).  

If we can find a suitable inexpensive check that has better semantics, I am 
fine to change this,



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145965

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

[PATCH] D147422: [clang-format] NFC Document the other space before colon option

[sstwcw via Phabricator via cfe-commits]

sstwcw created this revision.
sstwcw added reviewers: HazardyKnusperkeks, MyDeveloperDay, owenpan, rymiel.
Herald added subscribers: cfe-commits, JDevlieghere.
Herald added projects: All, clang, clang-format.
sstwcw requested review of this revision.
Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add you unit tests in `clang/unittests/Format` and build `ninja FormatTests` we 
recommend using the `verifyFormat(xxx)` format of unit tests rather than 
`EXPECT_EQ` as this will ensure you change is tolerant to random whitespace 
changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`


There are two options that do much the same thing, but for different
languages.  With the addition to the doc, the user is less likely to
configure the wrong option and get frustrated that it doesn't work.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147422

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3748,7 +3748,8 @@
   /// \version 7
   bool SpaceBeforeInheritanceColon;
 
-  /// If ``true``, a space will be add before a JSON colon.
+  /// If ``true``, a space will be add before a JSON colon. For other 
languages,
+  /// use ``SpacesInContainerLiterals`` instead.
   /// \code
   ///true:  false:
   ///{  {
@@ -4031,8 +4032,9 @@
   /// \version 10
   bool SpacesInConditionalStatement;
 
-  /// If ``true``, spaces are inserted inside container literals (e.g.
-  /// ObjC and Javascript array and dict literals).
+  /// If ``true``, spaces are inserted inside container literals (e.g.  ObjC 
and
+  /// Javascript array and dict literals). For JSON, use
+  /// ``SpaceBeforeJsonColon`` instead.
   /// \code{.js}
   ///true:  false:
   ///var arr = [ 1, 2, 3 ]; vs. var arr = [1, 2, 3];
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -4781,7 +4781,8 @@
 .. _SpaceBeforeJsonColon:
 
 **SpaceBeforeJsonColon** (``Boolean``) :versionbadge:`clang-format 17` :ref:`¶ 
`
-  If ``true``, a space will be add before a JSON colon.
+  If ``true``, a space will be add before a JSON colon. For other languages,
+  use ``SpacesInContainerLiterals`` instead.
 
   .. code-block:: c++
 
@@ -5099,8 +5100,9 @@
 .. _SpacesInContainerLiterals:
 
 **SpacesInContainerLiterals** (``Boolean``) :versionbadge:`clang-format 3.7` 
:ref:`¶ `
-  If ``true``, spaces are inserted inside container literals (e.g.
-  ObjC and Javascript array and dict literals).
+  If ``true``, spaces are inserted inside container literals (e.g.  ObjC and
+  Javascript array and dict literals). For JSON, use
+  ``SpaceBeforeJsonColon`` instead.
 
   .. code-block:: js
 


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3748,7 +3748,8 @@
   /// \version 7
   bool SpaceBeforeInheritanceColon;
 
-  /// If ``true``, a space will be add before a JSON colon.
+  /// If ``true``, a space will be add before a JSON colon. For other languages,
+  /// use ``SpacesInContainerLiterals`` instead.
   /// \code
   ///true:  false:
   ///{  {
@@ -4031,8 +4032,9 @@
   /// \version 10
   bool SpacesInConditionalStatement;
 
-  /// If ``true``, spaces are inserted inside container literals (e.g.
-  /// ObjC and Javascript array and dict literals).
+  /// If ``true``, spaces are inserted inside container literals (e.g.  ObjC and
+  /// Javascript array and dict literals). For JSON, use
+  /// ``SpaceBeforeJsonColon`` instead.
   /// \code{.js}
   ///true:  false:
   ///var arr = [ 1, 2, 3 ]; vs. var arr = [1, 2, 3];
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -4781,7 +4781,8 @@
 .. _SpaceBeforeJsonColon:
 
 **SpaceBeforeJsonColon** (``Boolean``) :versionbadge:`clang-format 17` :ref:`¶ `
- 

[PATCH] D146269: MIPS: allow o32 abi with 64bit CPU and 64 abi with 32bit triple

[YunQiang Su via Phabricator via cfe-commits]

wzssyqa added a comment.

ping


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

https://reviews.llvm.org/D146269

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

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaLookup.cpp:2082
+Module *DeclModule = SemaRef.getOwningModule(D);
+if (DeclModule && !DeclModule->isModuleMapModule() &&
+!SemaRef.isModuleUnitOfCurrentTU(DeclModule) &&

iains wrote:
> ChuanqiXu wrote:
> > We should check header units here.
> The point of checking module map modules was to avoid affecting clang modules 
> with the change in semantics.
> 
> At the moment, I am not sure why we could exclude lookup from finding decls 
> in an imported header unit?
> 
> At the moment, I am not sure why we could exclude lookup from finding decls 
> in an imported header unit?

We're not **excluding** decls from an imported header units. We're trying to 
include the decls from the headers units. Since we've allowed decls with 
internal linkage to appear to header units. We must need to allow decls with 
internal linkage in  header units to be found here. Otherwise, the following 
example may fail:

```
// foo.h
static int a = 43;

// m.cppm
export module m;
import "foo.h";
use of a...
```

BTW, given the semantics of header units and clang modules are pretty similar, 
we should use `isHeaderModule ` in most cases.



Comment at: clang/lib/Sema/SemaLookup.cpp:2101
+  if (isVisible(SemaRef, ND)) {
+if (SemaRef.getLangOpts().CPlusPlusModules && ND->isFromASTFile()) {
+  // The module that owns the decl is visible; However

iains wrote:
> ChuanqiXu wrote:
> > Let's not use `isFromASTFile()`. It is a low level API without higher level 
> > semantics. I think it is good enough to check the module of ND.
> lookup is very heavily used; the purpose of the isFromAST() check is to 
> short-circuit the more expensive checks when we know that a decl must be in 
> the same TU (i.e. it is not from an AST file).  
> 
> If we can find a suitable inexpensive check that has better semantics, I am 
> fine to change this,
> 
It looks good enough to me to check that `ND` lives in a module. And from what 
I profiled, the lookup process is not hot really.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145965

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

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

[Iain Sandoe via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaLookup.cpp:2082
+Module *DeclModule = SemaRef.getOwningModule(D);
+if (DeclModule && !DeclModule->isModuleMapModule() &&
+!SemaRef.isModuleUnitOfCurrentTU(DeclModule) &&

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > We should check header units here.
> > The point of checking module map modules was to avoid affecting clang 
> > modules with the change in semantics.
> > 
> > At the moment, I am not sure why we could exclude lookup from finding decls 
> > in an imported header unit?
> > 
> > At the moment, I am not sure why we could exclude lookup from finding decls 
> > in an imported header unit?
> 
> We're not **excluding** decls from an imported header units. We're trying to 
> include the decls from the headers units. Since we've allowed decls with 
> internal linkage to appear to header units. We must need to allow decls with 
> internal linkage in  header units to be found here. Otherwise, the following 
> example may fail:
> 
> ```
> // foo.h
> static int a = 43;
> 
> // m.cppm
> export module m;
> import "foo.h";
> use of a...
> ```
> 
> BTW, given the semantics of header units and clang modules are pretty 
> similar, we should use `isHeaderModule ` in most cases.
ah yes, that special case, I should have remembered.
OK I will adjust this at the next iteration (I think you mean 
`isHeaderLikeModule()`)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145965

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

[PATCH] D147419: [clang-tidy] ignore NRVO const variables in performance-no-automatic-move.

[Clement Courbet via Phabricator via cfe-commits]

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

Thanks for the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147419

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

[PATCH] D147326: [clang][dataflow][NFC] Share code between Environment ctor and pushCallInternal().

[Martin Böhme via Phabricator via cfe-commits]

mboehme updated this revision to Diff 510410.
mboehme added a comment.

Changes in response to review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147326

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -203,7 +203,33 @@
 
 // FIXME: Add support for resetting globals after function calls to enable
 // the implementation of sound analyses.
-void Environment::initVars(llvm::DenseSet Vars) {
+void Environment::initGlobalsAndFields(const FunctionDecl *FuncDecl) {
+  assert(FuncDecl->getBody() != nullptr);
+
+  llvm::DenseSet Fields;
+  llvm::DenseSet Vars;
+
+  // Look for global variable and field references in the
+  // constructor-initializers.
+  if (const auto *CtorDecl = dyn_cast(FuncDecl)) {
+for (const auto *Init : CtorDecl->inits()) {
+  if (const auto *M = Init->getAnyMember())
+  Fields.insert(M);
+  const Expr *E = Init->getInit();
+  assert(E != nullptr);
+  getFieldsAndGlobalVars(*E, Fields, Vars);
+}
+// Add all fields mentioned in default member initializers.
+for (const FieldDecl *F : CtorDecl->getParent()->fields())
+  if (const auto *I = F->getInClassInitializer())
+  getFieldsAndGlobalVars(*I, Fields, Vars);
+  }
+  getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
+
+  // These have to be added before the lines that follow to ensure that
+  // `create*` work correctly for structs.
+  DACtx->addModeledFields(Fields);
+
   for (const VarDecl *D : Vars) {
 if (getStorageLocation(*D, SkipPast::None) != nullptr)
   continue;
@@ -239,31 +265,7 @@
   if (const auto *FuncDecl = dyn_cast(&DeclCtx)) {
 assert(FuncDecl->getBody() != nullptr);
 
-llvm::DenseSet Fields;
-llvm::DenseSet Vars;
-
-// Look for global variable and field references in the
-// constructor-initializers.
-if (const auto *CtorDecl = dyn_cast(&DeclCtx)) {
-  for (const auto *Init : CtorDecl->inits()) {
-if (const auto *M = Init->getAnyMember())
-  Fields.insert(M);
-const Expr *E = Init->getInit();
-assert(E != nullptr);
-getFieldsAndGlobalVars(*E, Fields, Vars);
-  }
-  // Add all fields mentioned in default member initializers.
-  for (const FieldDecl *F  : CtorDecl->getParent()->fields())
-if (const auto *I = F->getInClassInitializer())
-  getFieldsAndGlobalVars(*I, Fields, Vars);
-}
-getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
-
-// These have to be added before the lines that follow to ensure that
-// `create*` work correctly for structs.
-DACtx.addModeledFields(Fields);
-
-initVars(Vars);
+initGlobalsAndFields(FuncDecl);
 
 for (const auto *ParamDecl : FuncDecl->parameters()) {
   assert(ParamDecl != nullptr);
@@ -337,26 +339,7 @@
ArrayRef Args) {
   CallStack.push_back(FuncDecl);
 
-  // FIXME: Share this code with the constructor, rather than duplicating it.
-  llvm::DenseSet Fields;
-  llvm::DenseSet Vars;
-  // Look for global variable references in the constructor-initializers.
-  if (const auto *CtorDecl = dyn_cast(FuncDecl)) {
-for (const auto *Init : CtorDecl->inits()) {
-  if (const auto *M = Init->getAnyMember())
-Fields.insert(M);
-  const Expr *E = Init->getInit();
-  assert(E != nullptr);
-  getFieldsAndGlobalVars(*E, Fields, Vars);
-}
-  }
-  getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
-
-  // These have to be added before the lines that follow to ensure that
-  // `create*` work correctly for structs.
-  DACtx->addModeledFields(Fields);
-
-  initVars(Vars);
+  initGlobalsAndFields(FuncDecl);
 
   const auto *ParamIt = FuncDecl->param_begin();
 
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -460,8 +460,9 @@
   void pushCallInternal(const FunctionDecl *FuncDecl,
 ArrayRef Args);
 
-  /// Assigns storage locations and values to all variables in `Vars`.
-  void initVars(llvm::DenseSet Vars);
+  /// Assigns storage locations and values to all global variables and fields
+  /// referenced in `FuncDecl`. `FuncDecl` must have a body.
+  void initGlobalsAndFields(const FunctionDecl *FuncDecl);
 
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
___
cfe