[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

2023-05-08 Thread Qingyuan Zheng via Phabricator via cfe-commits
daiyousei-qz added a comment.

Sorry, I'm a little occupied lately and don't have time to fix the test 
failure. I'll try to fix that in this week.




Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:400
+  bool ActiveKindLookup[static_cast(HighlightingKind::LastKind) + 1];
+  uint32_t ActiveModifiersMask;
+};

nridge wrote:
> For good measure, let's `static_assert(HighlightingModifier::LastModifier < 
> 32)`
IIRC, we already have such static_assert after the definition of 
HighlightingModifier in SemanticHighlight.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148489

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


[PATCH] D150089: [LoongArch] Support fcc* (condition flag) registers in inlineasm clobbers

2023-05-08 Thread WÁNG Xuěruì via Phabricator via cfe-commits
xen0n accepted this revision.
xen0n added a comment.

It's confirmed offline that LA32 also has 8 `$fcc`'s, so the existing 
assumption holds, and the code changes are correct. (Apparently the "LA32 
Reduced" subset is not supported right now, which only has one `$fcc0`.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150089

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


[PATCH] D149495: [RISCV] Add support for V extension in SiFive7

2023-05-08 Thread Wang Pengcheng via Phabricator via cfe-commits
pcwang-thead added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVSchedSiFive7.td:15
+class SiFive7IsWorstCaseMX MxList> {
+  string LLMUL = LargestLMUL.r;
+  bit c = !eq(mx, LLMUL);

I think I have fixed the issue that `defar` can't refer to template arguments 
in D148197. So `LMUL`, `SSEW` and other fields can be replaced with `defvar`s.



Comment at: llvm/lib/Target/RISCV/RISCVScheduleV.td:41
 
+// Helper function to get the largest LMUL from MxList
+// Precondition: MxList is sorted in ascending LMUL order.

So, are we going to discard `LMULXXXImpl` below?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149495

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


[PATCH] D150089: [LoongArch] Support fcc* (condition flag) registers in inlineasm clobbers

2023-05-08 Thread hev via Phabricator via cfe-commits
hev accepted this revision.
hev added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150089

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


[clang] 749b4ad - [clang] Modernize LoopHint (NFC)

2023-05-08 Thread Kazu Hirata via cfe-commits

Author: Kazu Hirata
Date: 2023-05-08T20:23:33-07:00
New Revision: 749b4ad315215534f0c6de2c9c732e1de750d8af

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

LOG: [clang] Modernize LoopHint (NFC)

Added: 


Modified: 
clang/include/clang/Parse/LoopHint.h

Removed: 




diff  --git a/clang/include/clang/Parse/LoopHint.h 
b/clang/include/clang/Parse/LoopHint.h
index 6e363f72b6587..75705fcd4c75c 100644
--- a/clang/include/clang/Parse/LoopHint.h
+++ b/clang/include/clang/Parse/LoopHint.h
@@ -23,20 +23,18 @@ struct LoopHint {
   // Identifier corresponding to the name of the pragma.  "loop" for
   // "#pragma clang loop" directives and "unroll" for "#pragma unroll"
   // hints.
-  IdentifierLoc *PragmaNameLoc;
+  IdentifierLoc *PragmaNameLoc = nullptr;
   // Name of the loop hint.  Examples: "unroll", "vectorize".  In the
   // "#pragma unroll" and "#pragma nounroll" cases, this is identical to
   // PragmaNameLoc.
-  IdentifierLoc *OptionLoc;
+  IdentifierLoc *OptionLoc = nullptr;
   // Identifier for the hint state argument.  If null, then the state is
   // default value such as for "#pragma unroll".
-  IdentifierLoc *StateLoc;
+  IdentifierLoc *StateLoc = nullptr;
   // Expression for the hint argument if it exists, null otherwise.
-  Expr *ValueExpr;
+  Expr *ValueExpr = nullptr;
 
-  LoopHint()
-  : PragmaNameLoc(nullptr), OptionLoc(nullptr), StateLoc(nullptr),
-ValueExpr(nullptr) {}
+  LoopHint() = default;
 };
 
 } // end namespace clang



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


[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

2023-05-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 520573.
MaskRay added a comment.

use `llvm::sys::path::stem(getDefaultImageName())`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149193

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/hip-gsplit-dwarf-options.hip
  clang/test/Driver/split-debug.c

Index: clang/test/Driver/split-debug.c
===
--- clang/test/Driver/split-debug.c
+++ clang/test/Driver/split-debug.c
@@ -9,6 +9,7 @@
 
 // INLINE: "-fsplit-dwarf-inlining"
 // NOINLINE-NOT: "-fsplit-dwarf-inlining"
+// SPLIT-NOT:  "-dumpdir"
 // SPLIT:  "-debug-info-kind=constructor"
 // SPLIT-SAME: "-ggnu-pubnames"
 // SPLIT-SAME: "-split-dwarf-file" "split-debug.dwo" "-split-dwarf-output" "split-debug.dwo"
@@ -54,8 +55,29 @@
 // SINGLE_WITH_FILENAME: "-split-dwarf-file" "{{.*}}foo.o"
 // SINGLE_WITH_FILENAME-NOT: "-split-dwarf-output"
 
-/// Without -c, clang performs linking as well. The output is unchanged.
-// RUN: %clang -### -target x86_64-unknown-linux-gnu -gsplit-dwarf -g %s -o ignore.d 2>&1 | FileCheck %s --check-prefix=SPLIT
+/// If linking is the final phase, the .dwo filename is derived from -o (if specified) or "a".
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -gsplit-dwarf -g %s -o obj/out 2>&1 | FileCheck %s --check-prefix=SPLIT_LINK
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -gsplit-dwarf -g %s 2>&1 | FileCheck %s --check-prefix=SPLIT_LINK_A
+
+// SPLIT_LINK:  "-dumpdir" "obj/out-"
+// SPLIT_LINK:  "-debug-info-kind=constructor"
+// SPLIT_LINK-SAME: "-split-dwarf-file" "obj/out-split-debug.dwo" "-split-dwarf-output" "obj/out-split-debug.dwo"
+// SPLIT_LINK_A:  "-dumpdir" "a-"
+// SPLIT_LINK_A-SAME: "-split-dwarf-file" "a-split-debug.dwo" "-split-dwarf-output" "a-split-debug.dwo"
+
+/// GCC special cases /dev/null (HOST_BIT_BUCKET) but not other special files like /dev/zero.
+/// We don't apply special rules at all.
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -gsplit-dwarf -g %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=SPLIT_LINK_NULL
+
+// SPLIT_LINK_NULL:  "-dumpdir" "/dev/null-"
+// SPLIT_LINK_NULL-SAME: "-split-dwarf-output" "/dev/null-split-debug.dwo"
+
+/// If -dumpdir is specified, use its value to derive the .dwo filename.
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -gsplit-dwarf -g %s -o obj/out -dumpdir pf/x -c 2>&1 | FileCheck %s --check-prefix=DUMPDIR
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -gsplit-dwarf -g %s -o obj/out -dumpdir pf/x 2>&1 | FileCheck %s --check-prefix=DUMPDIR
+
+// DUMPDIR:  "-dumpdir" "pf/x"
+// DUMPDIR-SAME: "-split-dwarf-output" "pf/xsplit-debug.dwo"
 
 /// -fsplit-dwarf-inlining
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=split -g -fsplit-dwarf-inlining %s 2>&1 | FileCheck %s --check-prefixes=INLINE,SPLIT
Index: clang/test/Driver/hip-gsplit-dwarf-options.hip
===
--- clang/test/Driver/hip-gsplit-dwarf-options.hip
+++ clang/test/Driver/hip-gsplit-dwarf-options.hip
@@ -13,13 +13,17 @@
 // RUN: %clang -### --target=x86_64-unknown-linux-gnu \
 // RUN:   --offload-arch=gfx906:xnack+ %s -nogpulib -nogpuinc \
 // RUN:   --offload-arch=gfx900 \
-// RUN:   -ggdb -gsplit-dwarf 2>&1 | FileCheck %s
+// RUN:   -ggdb -gsplit-dwarf 2>&1 | FileCheck %s --check-prefix=LINK
 
 // RUN: %clang -### --target=x86_64-unknown-linux-gnu \
 // RUN:   -fgpu-rdc --offload-arch=gfx906:xnack+ %s -nogpulib -nogpuinc \
 // RUN:   --offload-arch=gfx900 \
-// RUN:   -ggdb -gsplit-dwarf 2>&1 | FileCheck %s
+// RUN:   -ggdb -gsplit-dwarf 2>&1 | FileCheck %s --check-prefix=LINK
 
 // CHECK-DAG: {{".*clang.*".* "-target-cpu" "gfx906".* "-split-dwarf-output" "hip-gsplit-dwarf-options_gfx906:xnack\+.dwo"}}
 // CHECK-DAG: {{".*clang.*".* "-target-cpu" "gfx900".* "-split-dwarf-output" "hip-gsplit-dwarf-options_gfx900.dwo"}}
 // CHECK-DAG: {{".*clang.*".* "-target-cpu" "x86-64".* "-split-dwarf-output" "hip-gsplit-dwarf-options.dwo"}}
+
+// LINK-DAG: {{".*clang.*".* "-target-cpu" "gfx906".* "-split-dwarf-output" "a-hip-gsplit-dwarf-options_gfx906:xnack\+.dwo"}}
+// LINK-DAG: {{".*clang.*".* "-target-cpu" "gfx900".* "-split-dwarf-output" "a-hip-gsplit-dwarf-options_gfx900.dwo"}}
+// LINK-DAG: {{".*clang.*".* "-target-cpu" "x86-64".* "-split-dwarf-output" "a-hip-gsplit-dwarf-options.dwo"}}
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1248,23 +1248,24 @@
 if (StringRef(A->getValue()) == "single")
   return Args.MakeArgString(Output.getFilename());
 
-  Arg *FinalOutput = 

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

2023-05-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked 3 inline comments as done.
MaskRay added a comment.

In D149193#4328452 , @dblaikie wrote:

>> I agree that for most(all?) split DWARF users will not see any difference 
>> since they always use -c -o and don't combine compilation and linking in one 
>> command.
>
> Given that, I'm not sure that this is worth implementing, but if it suits you 
> I guess.

The split DWARF users are about production users.

There are command line users who do `clang -g -gsplit-dwarf a.c b.c -o x` and I 
have heard about questions that the `.dwo` files go to strange directories 
(usually `/tmp`) quite a few times.
Fixing `-gsplit-dwarf` helps set a baseline for other options like 
`-ftime-trace` (https://github.com/llvm/llvm-project/issues/57285).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149193

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


[PATCH] D150057: [clang-format] Fix consecutive alignments in #else blocks

2023-05-08 Thread Owen Pan via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa4c87f8ccacc: [clang-format] Fix consecutive alignments in 
#else blocks (authored by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D150057?vs=520137=520560#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150057

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2759,7 +2759,7 @@
 
   // Checks an edge case in preprocessor handling.
   // These comments should *not* be aligned
-  EXPECT_NE( // change for EQ when fixed
+  EXPECT_EQ(
   "#if FOO\n"
   "#else\n"
   "long a; // Line about a\n"
@@ -4316,7 +4316,7 @@
 )",
 format(R"(//
 /\
-/ 
+/
   )",
getLLVMStyleWithColumns(10)));
 }
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6367,6 +6367,51 @@
"#endif\n"
"}",
Style);
+
+  verifyFormat("#if FOO\n"
+   "int a = 1;\n"
+   "#else\n"
+   "int ab = 2;\n"
+   "#endif\n"
+   "#ifdef BAR\n"
+   "int abc = 3;\n"
+   "#elifdef BAZ\n"
+   "int abcd = 4;\n"
+   "#endif",
+   Style);
+
+  verifyFormat("void f() {\n"
+   "  if (foo) {\n"
+   "#if FOO\n"
+   "int a = 1;\n"
+   "#else\n"
+   "bool a = true;\n"
+   "#endif\n"
+   "int abc = 3;\n"
+   "#ifndef BAR\n"
+   "int abcd = 4;\n"
+   "#elif BAZ\n"
+   "bool abcd = true;\n"
+   "#endif\n"
+   "  }\n"
+   "}",
+   Style);
+
+  verifyFormat("void f() {\n"
+   "#if FOO\n"
+   "  a = 1;\n"
+   "#else\n"
+   "  ab = 2;\n"
+   "#endif\n"
+   "}\n"
+   "void g() {\n"
+   "#if BAR\n"
+   "  abc = 3;\n"
+   "#elifndef BAZ\n"
+   "  abcd = 4;\n"
+   "#endif\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, FormatHashIfNotAtStartOfLine) {
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -49,8 +49,16 @@
   unsigned Spaces,
   unsigned StartOfTokenColumn,
   bool IsAligned, bool InPPDirective) {
-  if (Tok.Finalized || (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg))
+  auto PPBranchDirectiveStartsLine = [] {
+return Tok.is(tok::hash) && !Tok.Previous && Tok.Next &&
+   Tok.Next->isOneOf(tok::pp_if, tok::pp_ifdef, tok::pp_ifndef,
+ tok::pp_elif, tok::pp_elifdef, tok::pp_elifndef,
+ tok::pp_else, tok::pp_endif);
+  };
+  if ((Tok.Finalized && !PPBranchDirectiveStartsLine()) ||
+  (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg)) {
 return;
+  }
   Tok.setDecision((Newlines > 0) ? FD_Break : FD_Continue);
   Changes.push_back(Change(Tok, /*CreateReplacement=*/true, Tok.WhitespaceRange,
Spaces, StartOfTokenColumn, Newlines, "", "",
@@ -522,13 +530,6 @@
? Changes[StartAt].indentAndNestingLevel()
: std::tuple();
 
-  // Keep track if the first token has a non-zero indent and nesting level.
-  // This can happen when aligning the contents of "#else" preprocessor blocks,
-  // which is done separately.
-  bool HasInitialIndentAndNesting =
-  StartAt == 0 &&
-  IndentAndNestingLevel > std::tuple();
-
   // Keep track of the number of commas before the matching tokens, we will only
   // align a sequence of matching tokens if they are preceded by the same number
   // of commas.
@@ -563,19 +564,8 @@
 
   unsigned i = StartAt;
   for (unsigned e = Changes.size(); i != e; ++i) {
-if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel) {
-  if (!HasInitialIndentAndNesting)
-break;
-  // The contents of preprocessor blocks are aligned separately.
-  // If the initial preprocessor block is indented or nested (e.g. it's in
-  // a function), do not align and exit after 

[clang] a4c87f8 - [clang-format] Fix consecutive alignments in #else blocks

2023-05-08 Thread Owen Pan via cfe-commits

Author: Owen Pan
Date: 2023-05-08T19:45:30-07:00
New Revision: a4c87f8ccaccc76fd7d1c6c2e639ca84b9ec7794

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

LOG: [clang-format] Fix consecutive alignments in #else blocks

Since 3.8 or earlier, clang-format has been lumping all #else, #elif,
etc blocks together when doing whitespace replacements and causing
consecutive alignments across #else blocks.

Commit c077975 partially addressed the problem but also triggered
"regressions".

This patch fixes the root cause of the problem and "reverts" c077975
(except for the unit tests).

Fixes #36070.
Fixes #55265.
Fixes #60721.
Fixes #61498.

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

Added: 


Modified: 
clang/lib/Format/WhitespaceManager.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/FormatTestComments.cpp

Removed: 




diff  --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index d2be3b3a77df4..040b9536b6305 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -49,8 +49,16 @@ void WhitespaceManager::replaceWhitespace(FormatToken , 
unsigned Newlines,
   unsigned Spaces,
   unsigned StartOfTokenColumn,
   bool IsAligned, bool InPPDirective) {
-  if (Tok.Finalized || (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg))
+  auto PPBranchDirectiveStartsLine = [] {
+return Tok.is(tok::hash) && !Tok.Previous && Tok.Next &&
+   Tok.Next->isOneOf(tok::pp_if, tok::pp_ifdef, tok::pp_ifndef,
+ tok::pp_elif, tok::pp_elifdef, tok::pp_elifndef,
+ tok::pp_else, tok::pp_endif);
+  };
+  if ((Tok.Finalized && !PPBranchDirectiveStartsLine()) ||
+  (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg)) {
 return;
+  }
   Tok.setDecision((Newlines > 0) ? FD_Break : FD_Continue);
   Changes.push_back(Change(Tok, /*CreateReplacement=*/true, 
Tok.WhitespaceRange,
Spaces, StartOfTokenColumn, Newlines, "", "",
@@ -522,13 +530,6 @@ static unsigned AlignTokens(const FormatStyle , F 
&,
? Changes[StartAt].indentAndNestingLevel()
: std::tuple();
 
-  // Keep track if the first token has a non-zero indent and nesting level.
-  // This can happen when aligning the contents of "#else" preprocessor blocks,
-  // which is done separately.
-  bool HasInitialIndentAndNesting =
-  StartAt == 0 &&
-  IndentAndNestingLevel > std::tuple();
-
   // Keep track of the number of commas before the matching tokens, we will 
only
   // align a sequence of matching tokens if they are preceded by the same 
number
   // of commas.
@@ -563,19 +564,8 @@ static unsigned AlignTokens(const FormatStyle , F 
&,
 
   unsigned i = StartAt;
   for (unsigned e = Changes.size(); i != e; ++i) {
-if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel) {
-  if (!HasInitialIndentAndNesting)
-break;
-  // The contents of preprocessor blocks are aligned separately.
-  // If the initial preprocessor block is indented or nested (e.g. it's in
-  // a function), do not align and exit after finishing this scope block.
-  // Instead, align, and then lower the baseline indent and nesting level
-  // in order to continue aligning subsequent blocks.
-  EndOfSequence = i;
-  AlignCurrentSequence();
-  IndentAndNestingLevel =
-  Changes[i].indentAndNestingLevel(); // new baseline
-}
+if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel)
+  break;
 
 if (Changes[i].NewlinesBefore != 0) {
   CommasBeforeMatch = 0;

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 099a5cb913643..dc673934a3f1b 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -6367,6 +6367,51 @@ TEST_F(FormatTest, 
FormatAlignInsidePreprocessorElseBlock) {
"#endif\n"
"}",
Style);
+
+  verifyFormat("#if FOO\n"
+   "int a = 1;\n"
+   "#else\n"
+   "int ab = 2;\n"
+   "#endif\n"
+   "#ifdef BAR\n"
+   "int abc = 3;\n"
+   "#elifdef BAZ\n"
+   "int abcd = 4;\n"
+   "#endif",
+   Style);
+
+  verifyFormat("void f() {\n"
+   "  if (foo) {\n"
+   "#if FOO\n"
+   "int a = 1;\n"
+   "#else\n"
+   "bool a = true;\n"
+   "#endif\n"
+   "

[PATCH] D150057: [clang-format] Fix consecutive alignments in #else blocks

2023-05-08 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:6394
+   "#elif BAZ\n"
+   "bool ab = true;\n"
+   "#endif\n"




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150057

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


[PATCH] D148457: [clangd] Support macro evaluation on hover

2023-05-08 Thread Younan Zhang via Phabricator via cfe-commits
zyounan added a comment.

Thank you for the opinions. I've updated and please take a look.




Comment at: clang-tools-extra/clangd/Hover.cpp:705
+
+  // If macro expands to one single token, rule out punctuator or digraph.
+  // E.g., for the case `array L_BRACKET 42 R_BRACKET;` where L_BRACKET and

nridge wrote:
> This check for punctuators actually makes the behaviour **more strict** for 
> macros than for non-macros in some cases.
> 
> For example:
> 
> ```
> #define PLUS +
> 
> constexpr int A = 2 + 3;  // hover over `+` shows `Value = 5`
> constexpr int B = 2 PLUS 3;  // hover over `PLUS` does not show `Value`
> ```
> 
> I don't think this is a particularly important use case (it's a bit of a hack 
> to get the expression's value this way, much better would be to select the 
> entire expression you want to evaluate in the editor, but unfortunately LSP 
> only sends a single position in `textDocument/hover`, not a full range...), 
> but perhaps we could consider relaxing this in the future. (I'm thinking, if 
> we switched to "allow partial selection via macros that expand to a single 
> token", we could probably drop this condition.)
Thanks for clarifying this intention (for any bug hunter in the future).
> if we switched to "allow partial selection via macros that expand to a single 
> token", we could probably drop this condition.

I'm afraid we can't. For the array case in the test,
```
vector left_b^racket 3 right_b^racket;
// left_bracket -> [
// right_bracket -> ]
```
the associated selection tree is,
```
 TranslationUnitDecl 
   FunctionDecl void function()
 CompoundStmt { …
  .ArraySubscriptExpr vector[3]
```
(`function` is the wrapper that facilitates writing single expression, doesn't 
matter here.)
It expands to one single token and is partially selected, which exactly matches 
the strategy. If we do allow evaluation, we'd get a value/type on `[` or `]`. 
Yes, it is a contrived and weird use case, but for now I think it's fine to 
keep it unevaluated to avoid issue aforementioned.

(But I'm willing to remove this restriction if you prefer a relaxed strategy.)



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:3832
+  };
+  Alig$3^nOf(Y);
+)cpp",

nridge wrote:
> I guess with this style of test you can simplify the `$3^` to `^`
Aha, that's a typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148457

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


[PATCH] D148457: [clangd] Support macro evaluation on hover

2023-05-08 Thread Younan Zhang via Phabricator via cfe-commits
zyounan updated this revision to Diff 520555.
zyounan marked 2 inline comments as done.
zyounan added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148457

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -18,6 +18,7 @@
 #include "clang/Format/Format.h"
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
 
 #include "gtest/gtest.h"
 #include 
@@ -529,6 +530,8 @@
[](HoverInfo ) {
  HI.Name = "MACRO";
  HI.Kind = index::SymbolKind::Macro;
+ HI.Value = "41 (0x29)";
+ HI.Type = "int";
  HI.Definition = "#define MACRO 41\n\n"
  "// Expands to\n"
  "41";
@@ -560,6 +563,7 @@
[](HoverInfo ) {
  HI.Name = "DECL_STR";
  HI.Kind = index::SymbolKind::Macro;
+ HI.Type = HoverInfo::PrintedType("const char *");
  HI.Definition = "#define DECL_STR(NAME, VALUE) const char *v_##NAME = "
  "STRINGIFY(VALUE)\n\n"
  "// Expands to\n"
@@ -1792,6 +1796,8 @@
   )cpp",
   [](HoverInfo ) {
 HI.Name = "MACRO";
+HI.Value = "0";
+HI.Type = "int";
 HI.Kind = index::SymbolKind::Macro;
 HI.Definition = "#define MACRO 0\n\n"
 "// Expands to\n"
@@ -3746,6 +3752,232 @@
   EXPECT_EQ(H->Type->Type, "int");
   EXPECT_EQ(H->Definition, "using foo = type");
 }
+
+TEST(Hover, EvaluateMacros) {
+  llvm::StringRef PredefinedCXX = R"cpp(
+#define X 42
+#define SizeOf sizeof
+#define AlignOf alignof
+#define PLUS_TWO +2
+#define TWO 2
+
+using u64 = unsigned long long;
+// calculate (a ** b) % p
+constexpr u64 pow_with_mod(u64 a, u64 b, u64 p) {
+  u64 ret = 1;
+  while (b) {
+if (b & 1)
+  ret = (ret * a) % p;
+a = (a * a) % p;
+b >>= 1;
+  }
+  return ret;
+}
+#define last_n_digit(x, y, n)  \
+  pow_with_mod(x, y, pow_with_mod(10, n, 2147483647))
+#define declare_struct(X, name, value) \
+  struct X {   \
+constexpr auto name() { return value; }\
+  }
+#define gnu_statement_expression(value)\
+  ({   \
+declare_struct(Widget, getter, value); \
+Widget().getter(); \
+  })
+#define define_lambda_begin(lambda, ...)   \
+  [&](__VA_ARGS__) {
+#define define_lambda_end() }
+
+#define left_bracket [
+#define right_bracket ]
+#define dg_left_bracket <:
+#define dg_right_bracket :>
+#define array_decl(type, name, size) type name left_bracket size right_bracket
+  )cpp";
+
+  struct {
+llvm::StringRef Code;
+const std::function, size_t /*Id*/)>
+Validator;
+  } Cases[] = {
+  {
+  /*Code=*/R"cpp(
+X^;
+  )cpp",
+  /*Validator=*/
+  [](std::optional HI, size_t) {
+EXPECT_EQ(HI->Value, "42 (0x2a)");
+EXPECT_EQ(HI->Type, HoverInfo::PrintedType("int"));
+  },
+  },
+  {
+  /*Code=*/R"cpp(
+Size^Of(int);
+  )cpp",
+  /*Validator=*/
+  [](std::optional HI, size_t) {
+EXPECT_TRUE(HI->Value);
+EXPECT_TRUE(HI->Type);
+// Don't validate type or value of `sizeof` and `alignof` as we're
+// getting different values or desugared types on different
+// platforms. Same as below.
+  },
+  },
+  {
+  /*Code=*/R"cpp(
+  struct Y {
+int y;
+double z;
+  };
+  Alig^nOf(Y);
+)cpp",
+  /*Validator=*/
+  [](std::optional HI, size_t) {
+EXPECT_TRUE(HI->Value);
+EXPECT_TRUE(HI->Type);
+  },
+  },
+  {
+  /*Code=*/R"cpp(
+  // 2**32 == 4294967296
+  last_n_di^git(2, 32, 6);
+)cpp",
+  /*Validator=*/
+  [](std::optional HI, size_t) {
+EXPECT_EQ(HI->Value, "967296 (0xec280)");
+EXPECT_EQ(HI->Type, "u64");
+  },
+  },
+  {
+  /*Code=*/R"cpp(
+  gnu_statement_exp^ression(42);
+)cpp",
+  /*Validator=*/
+  [](std::optional HI, size_t) {
+

[PATCH] D150043: [InferAddressSpaces] Handle vector of pointers type & Support intrinsic masked gather/scatter

2023-05-08 Thread CaprYang via Phabricator via cfe-commits
CaprYang added inline comments.



Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:259-265
+static unsigned getPtrOrVecOfPtrsAddressSpace(Type *Ty) {
+  if (Ty->isVectorTy()) {
+Ty = cast(Ty)->getElementType();
+  }
+  assert(Ty->isPointerTy());
+  return Ty->getPointerAddressSpace();
+}

arsenm wrote:
> This is reinventing Type::getPointerAddressSpace
Yes.. it seems like I am doing some useless work.



Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:268-271
+  if (Ty->isVectorTy()) {
+Ty = cast(Ty)->getElementType();
+  }
+  return Ty->isPointerTy();

arsenm wrote:
> isPtrOrPtrVectorTy
Oh thanks! I just learned that there are these APIs, let me replace them.



Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:285
+  Type *NPT =
+  PointerType::getWithSamePointeeType(cast(PT), NewAddrSpace);
+  return VectorType::get(NPT, cast(Ty)->getElementCount());

arsenm wrote:
> Don't need to bother trying to maintain pointer element types anymore 
Sorry.. I don't know what to do, can you tell me?



Comment at: llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll:151
 ; CHECK-LABEL: @icmp_flat_flat_from_group_vector(
-; CHECK: %cmp = icmp eq <2 x ptr> %cast0, %cast1
+; CHECK: %cmp = icmp eq <2 x ptr addrspace(3)> %group.ptr.0, %group.ptr.1
 define <2 x i1> @icmp_flat_flat_from_group_vector(<2 x ptr addrspace(3)> 
%group.ptr.0, <2 x ptr addrspace(3)> %group.ptr.1) #0 {

arsenm wrote:
> You touched a lot more than just icmp, so this needs more tests to cover all 
> the newly handled cases 
I will add more tests later


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

https://reviews.llvm.org/D150043

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


[PATCH] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

2023-05-08 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment.

In D148776#4328425 , @dblaikie wrote:

> Got a link to a design discussion motivating this change?

No design discussion. I though that doing less work is not contentious.

> I'd have thought it made sense to put modulemaps in subdirectories - since 
> they cover the whole directory, putting them in the root of an include path 
> would be problematic if there are multiple distinct projects in that same 
> path. And I didn't think this involved searching through subdirectories, but 
> walking up parent directories from the included file...

When we look for a module map covering a header - you are right, we are walking 
up parent directories from the included file. But when we are looking up a 
module by name (for example, when we load a module), there is no included file 
and we need to look for a module top-down instead of bottom-up.

Your suggested approach to put module maps in corresponding subdirectories 
works when the names are the same, for example, module "blue" in directory 
"blue", that remains unchanged. But, for example, module "LLVM_DebugInfo" in 
directory "llvm" is not obvious. And there is no indication it //must// be in 
directory "llvm" and not in "llvm-c" or in some other directory, so clang 
checks all subdirectories.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148776

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


[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

2023-05-08 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

> I agree that for most(all?) split DWARF users will not see any difference 
> since they always use -c -o and don't combine compilation and linking in one 
> command.

Given that, I'm not sure that this is worth implementing, but if it suits you I 
guess.




Comment at: clang/lib/Driver/Driver.cpp:3884
+  nullptr, getOpts().getOption(options::OPT_dumpdir),
+  Args.MakeArgString(Args.getLastArgValue(options::OPT_o, "a") + "-"));
+  Arg->claim();

MaskRay wrote:
> scott.linder wrote:
> > MaskRay wrote:
> > > dblaikie wrote:
> > > > would be nice to have this "a" derive from wherever we hardcode "a.out" 
> > > > as the default output rather than independently hardcoded here?
> > > > 
> > > > & what does GCC do when the `-o` value has a `.` in it? (if you use `-o 
> > > > a.out` do you get the same `a-x.dwo` behavior, or do you get 
> > > > `a.out-x.dwo`?)
> > > We can use `llvm::sys::path::stem(getDefaultImageName())`, but I feel 
> > > that this just complicates the code.
> > > The default is `a.out` or `a.exe`. If a downstream platform decides to 
> > > deviate and use another filename, say, `b.out`. We will use `-dumpdir b-` 
> > > on this platform and `-dumpdir a-` on everything else. I think they will 
> > > likely be fine with `a-` even if they don't use `a` as the stem name of 
> > > the default image...
> > > 
> > > GCC generally doesn't special case `.` in `-o` for linking, but the 
> > > `a.out` filename is different.
> > > 
> > > ```
> > > gcc -g -gsplit-dwarf d/a.c.c -o e/x.out  # e/x.out-a.dwo
> > > gcc -g -gsplit-dwarf d/a.c.c -o e/a.out  # e/a-a.dwo
> > > ```
> > > 
> > > I think Clang should not special case `a.out`.
> > GCC distinguishes between "dump" and "auxiliary" outputs, and in this case 
> > I think the "dump" outputs retain the basename-suffix (i.e. you get 
> > a.out) whereas "auxiliary" outputs first strip the basename-suffix 
> > (i.e. you get a). The basename-suffix itself can be specified 
> > explicitly via -dumpbase-ext, but it is inferred by default.
> > 
> > The naming for things adds to the for me:
> > 
> > * `-dumpdir` doesn't specifically/exclusively specify a "directory", it 
> > just specifies a prefix
> > * `-dumpbase-ext` only affects the output of non-dump, auxiliary files
> > 
> > I do worry that being close-but-not-quite like GCC here will cause 
> > headaches for someone, but I am also not excited about implementing the 
> > complexity of the GCC options.
> > ... I think the "dump" outputs retain the basename-suffix (i.e. you get 
> > a.out) whereas "auxiliary" outputs first strip the basename-suffix 
> > (i.e. you get a).
> 
> Confirmed.
> ```
> gcc -g -fdump-rtl-all -gsplit-dwarf d/a.c -o e/x.out
> ls e/x.out-a.c.338r.dfinish e/x.out-a.dwo
> ```
> 
> For dump output files, I think they all reside in developer options 
> (https://gcc.gnu.org/onlinedocs/gcc/Developer-Options.html) not intended to 
> be used by end users. They occasionally make incompatible changes in this 
> area as well, e.g. 
> https://gcc.gnu.org/pipermail/gcc-patches/2020-May/546009.html simplified 
> some rules.
> 
> It seems that if we just implement `-dumpdir`, we have gotten the good parts 
> and we are probably done :)
> We can use llvm::sys::path::stem(getDefaultImageName()), but I feel that this 
> just complicates the code.

I think it'd be a bit better this way - otherways "a" looks pretty arbitrary. 
Is there some other reason it would be "a"? If it is derived from "a.out" I 
think having the code reflect that is helpful to the reader.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149193

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


[clang] e494ebf - [OpenMP] Fix incorrect interop type for number of dependencies

2023-05-08 Thread Joseph Huber via cfe-commits

Author: Joseph Huber
Date: 2023-05-08T21:02:43-05:00
New Revision: e494ebf9d09b1112dcf4f22984bdb51bbf5d8cd7

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

LOG: [OpenMP] Fix incorrect interop type for number of dependencies

The interop types use the number of dependencies in the function
interface. Every other function uses an `i32` to count the number of
dependencies except for the initialization function. This leads to
codegen issues when the rest of the compiler passes in an `i32` that
then creates an invalid call. Fix this to be consistent with the other
uses.

Reviewed By: tianshilei1992

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

Added: 


Modified: 
clang/test/OpenMP/interop_irbuilder.cpp
llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/test/Transforms/OpenMP/add_attributes.ll
openmp/libomptarget/src/interop.cpp

Removed: 




diff  --git a/clang/test/OpenMP/interop_irbuilder.cpp 
b/clang/test/OpenMP/interop_irbuilder.cpp
index 337af16c5c90a..a0e666955ab11 100644
--- a/clang/test/OpenMP/interop_irbuilder.cpp
+++ b/clang/test/OpenMP/interop_irbuilder.cpp
@@ -10,23 +10,17 @@ void test1() {
   int D0, D1;
   omp_interop_t interop;
 
-#pragma omp interop init(target \
- : interop)
+#pragma omp interop init(target : interop)
 
-#pragma omp interop init(targetsync \
- : interop)
+#pragma omp interop init(targetsync : interop)
 
-#pragma omp interop init(target \
- : interop) device(device_id)
+#pragma omp interop init(target : interop) device(device_id)
 
-#pragma omp interop init(targetsync \
- : interop) device(device_id)
+#pragma omp interop init(targetsync : interop) device(device_id)
 
-#pragma omp interop use(interop) depend(in \
-: D0, D1) nowait
+#pragma omp interop use(interop) depend(in : D0, D1) nowait
 
-#pragma omp interop destroy(interop) depend(in \
-: D0, D1)
+#pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 
 struct S {
@@ -39,23 +33,17 @@ void S::member_test() {
   int device_id = 4;
   int D0, D1;
 
-#pragma omp interop init(target \
- : interop)
+#pragma omp interop init(target : interop)
 
-#pragma omp interop init(targetsync \
- : interop)
+#pragma omp interop init(targetsync : interop)
 
-#pragma omp interop init(target \
- : interop) device(device_id)
+#pragma omp interop init(target : interop) device(device_id)
 
-#pragma omp interop init(targetsync \
- : interop) device(device_id)
+#pragma omp interop init(targetsync : interop) device(device_id)
 
-#pragma omp interop use(interop) depend(in \
-: D0, D1) nowait
+#pragma omp interop use(interop) depend(in : D0, D1) nowait
 
-#pragma omp interop destroy(interop) depend(in \
-: D0, D1)
+#pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 // CHECK-LABEL: @_Z5test1v(
 // CHECK-NEXT:  entry:
@@ -69,15 +57,15 @@ void S::member_test() {
 // CHECK-NEXT:[[DEP_COUNTER_ADDR6:%.*]] = alloca i64, align 8
 // CHECK-NEXT:store i32 4, ptr [[DEVICE_ID]], align 4
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 
@__kmpc_global_thread_num(ptr @[[GLOB1:[0-9]+]])
-// CHECK-NEXT:call void @__tgt_interop_init(ptr @[[GLOB1]], i32 
[[OMP_GLOBAL_THREAD_NUM]], ptr [[INTEROP]], i32 1, i32 -1, i64 0, ptr null, i32 
0)
+// CHECK-NEXT:call void @__tgt_interop_init(ptr @[[GLOB1]], i32 
[[OMP_GLOBAL_THREAD_NUM]], ptr [[INTEROP]], i32 1, i32 -1, i32 0, ptr null, i32 
0)
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM1:%.*]] = call i32 
@__kmpc_global_thread_num(ptr @[[GLOB1]])
-// CHECK-NEXT:call void @__tgt_interop_init(ptr @[[GLOB1]], i32 
[[OMP_GLOBAL_THREAD_NUM1]], ptr [[INTEROP]], i32 2, i32 -1, i64 0, ptr null, 
i32 0)
+// CHECK-NEXT:call void @__tgt_interop_init(ptr @[[GLOB1]], i32 
[[OMP_GLOBAL_THREAD_NUM1]], ptr [[INTEROP]], i32 2, i32 -1, i32 0, ptr null, 
i32 0)
 // CHECK-NEXT:[[TMP0:%.*]] = load i32, ptr [[DEVICE_ID]], align 4
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM2:%.*]] = call i32 
@__kmpc_global_thread_num(ptr @[[GLOB1]])
-// CHECK-NEXT:call void @__tgt_interop_init(ptr @[[GLOB1]], i32 
[[OMP_GLOBAL_THREAD_NUM2]], ptr [[INTEROP]], i32 1, i32 [[TMP0]], i64 0, ptr 
null, i32 0)
+// CHECK-NEXT:call void @__tgt_interop_init(ptr @[[GLOB1]], i32 
[[OMP_GLOBAL_THREAD_NUM2]], ptr [[INTEROP]], i32 1, i32 [[TMP0]], i32 0, ptr 
null, i32 0)
 // CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[DEVICE_ID]], align 4
 // CHECK-NEXT:

[PATCH] D150156: [OpenMP] Fix incorrect interop type for number of dependencies

2023-05-08 Thread Joseph Huber 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 rGe494ebf9d09b: [OpenMP] Fix incorrect interop type for number 
of dependencies (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150156

Files:
  clang/test/OpenMP/interop_irbuilder.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/test/Transforms/OpenMP/add_attributes.ll
  openmp/libomptarget/src/interop.cpp

Index: openmp/libomptarget/src/interop.cpp
===
--- openmp/libomptarget/src/interop.cpp
+++ openmp/libomptarget/src/interop.cpp
@@ -184,7 +184,7 @@
 void __tgt_interop_init(ident_t *LocRef, kmp_int32 Gtid,
 omp_interop_val_t *,
 kmp_interop_type_t InteropType, kmp_int32 DeviceId,
-kmp_int64 Ndeps, kmp_depend_info_t *DepList,
+kmp_int32 Ndeps, kmp_depend_info_t *DepList,
 kmp_int32 HaveNowait) {
   kmp_int32 NdepsNoalias = 0;
   kmp_depend_info_t *NoaliasDepList = NULL;
Index: llvm/test/Transforms/OpenMP/add_attributes.ll
===
--- llvm/test/Transforms/OpenMP/add_attributes.ll
+++ llvm/test/Transforms/OpenMP/add_attributes.ll
@@ -742,7 +742,7 @@
 
 declare void @__tgt_interop_destroy(ptr, i32, ptr, i32, i32, ptr, i32);
 
-declare void @__tgt_interop_init(ptr, i32, ptr, i32, i32, i64, ptr, i32);
+declare void @__tgt_interop_init(ptr, i32, ptr, i32, i32, i32, ptr, i32);
 
 declare void @__tgt_interop_use(ptr, i32, ptr, i32, i32, ptr, i32);
 
@@ -1398,7 +1398,7 @@
 ; CHECK: declare void @__tgt_interop_destroy(ptr, i32, ptr, i32, i32, ptr, i32)
 
 ; CHECK-NOT: Function Attrs
-; CHECK: declare void @__tgt_interop_init(ptr, i32, ptr, i32, i32, i64, ptr, i32)
+; CHECK: declare void @__tgt_interop_init(ptr, i32, ptr, i32, i32, i32, ptr, i32)
 
 ; CHECK-NOT: Function Attrs
 ; CHECK: declare void @__tgt_interop_use(ptr, i32, ptr, i32, i32, ptr, i32)
@@ -2046,7 +2046,7 @@
 ; OPTIMISTIC: declare void @__tgt_interop_destroy(ptr, i32, ptr, i32, i32, ptr, i32)
 
 ; OPTIMISTIC-NOT: Function Attrs
-; OPTIMISTIC: declare void @__tgt_interop_init(ptr, i32, ptr, i32, i32, i64, ptr, i32)
+; OPTIMISTIC: declare void @__tgt_interop_init(ptr, i32, ptr, i32, i32, i32, ptr, i32)
 
 ; OPTIMISTIC-NOT: Function Attrs
 ; OPTIMISTIC: declare void @__tgt_interop_use(ptr, i32, ptr, i32, i32, ptr, i32)
@@ -2707,7 +2707,7 @@
 ; EXT: declare void @__tgt_interop_destroy(ptr, i32 signext, ptr, i32 signext, i32 signext, ptr, i32 signext)
 
 ; EXT-NOT: Function Attrs
-; EXT: declare void @__tgt_interop_init(ptr, i32 signext, ptr, i32 signext, i32 signext, i64, ptr, i32 signext)
+; EXT: declare void @__tgt_interop_init(ptr, i32 signext, ptr, i32 signext, i32 signext, i32, ptr, i32 signext)
 
 ; EXT-NOT: Function Attrs
 ; EXT: declare void @__tgt_interop_use(ptr, i32 signext, ptr, i32 signext, i32 signext, ptr, i32 signext)
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -3798,7 +3798,7 @@
 Device = ConstantInt::get(Int32, -1);
   Constant *InteropTypeVal = ConstantInt::get(Int32, (int)InteropType);
   if (NumDependences == nullptr) {
-NumDependences = ConstantInt::get(Int64, 0);
+NumDependences = ConstantInt::get(Int32, 0);
 PointerType *PointerTypeVar = Type::getInt8PtrTy(M.getContext());
 DependenceAddress = ConstantPointerNull::get(PointerTypeVar);
   }
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -402,7 +402,7 @@
 __OMP_RTL(__kmpc_free, false, Void, /* Int */ Int32, VoidPtr, VoidPtr)
 
 __OMP_RTL(__tgt_interop_init, false, Void, IdentPtr, Int32, VoidPtrPtr, Int32,
-  Int32, Int64, VoidPtr, Int32)
+  Int32, Int32, VoidPtr, Int32)
 __OMP_RTL(__tgt_interop_destroy, false, Void, IdentPtr, Int32, VoidPtrPtr,
   Int32, Int32, VoidPtr, Int32)
 __OMP_RTL(__tgt_interop_use, false, Void, IdentPtr, Int32, VoidPtrPtr, Int32,
Index: clang/test/OpenMP/interop_irbuilder.cpp
===
--- clang/test/OpenMP/interop_irbuilder.cpp
+++ clang/test/OpenMP/interop_irbuilder.cpp
@@ -10,23 +10,17 @@
   int D0, D1;
   omp_interop_t interop;
 
-#pragma omp interop init(target \
- : interop)
+#pragma omp interop init(target : interop)
 
-#pragma omp interop init(targetsync \
- : interop)
+#pragma omp interop init(targetsync : interop)
 

[PATCH] D150001: [clang] Fix initializer_list matching failures with modules

2023-05-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision.
ChuanqiXu added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150001

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


[PATCH] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

2023-05-08 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

Got a link to a design discussion motivating this change?

I'd have thought it made sense to put modulemaps in subdirectories - since they 
cover the whole directory, putting them in the root of an include path would be 
problematic if there are multiple distinct projects in that same path. And I 
didn't think this involved searching through subdirectories, but walking up 
parent directories from the included file...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148776

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


[PATCH] D149495: [RISCV] Add support for V extension in SiFive7

2023-05-08 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVSchedSiFive7.td:112
+  // Note: c >= 1 since the smallest VLUpperBound is 512 / 8 = 8, and the
+  // largest division performed on  VLUpperBound is in MF8 case with division
+  // by 8. Therefore, there is no need to ceil the result.

There's an extra space before VLUpperBound here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149495

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


[PATCH] D149642: [RISCV] Support vreinterpret intrinsics between vector boolean type and m1 vector integer type

2023-05-08 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:2042
+
+  SmallVector Operands;
+  if (ResultType->isIntOrIntVectorTy(1)) {

Don't use SmallVector for a fixed number of items. You can use a plain array.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149642

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


[PATCH] D148851: Disable llvm-symbolizer on some of the driver tests that are timing out

2023-05-08 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

In D148851#4328345 , @shafik wrote:

> In D148851#4328084 , @dblaikie 
> wrote:
>
>> In D148851#4311266 , @ahatanak 
>> wrote:
>>
>>> Disable llvm-symbolizer when running lit tests.
>>
>> This seems problematic though - when lit tests fail it's quite helpful to 
>> get a symbolized stack trace.
>
> While I have seen this again just today with the driver tests and it is 
> annoying, I would also prefer a more targeted solution if possible.

OK, I can go back to the original solution if it's important to keep the 
symbolizer enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148851

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


[PATCH] D150156: [OpenMP] Fix incorrect interop type for number of dependencies

2023-05-08 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 520543.
jhuber6 added a comment.

Fix `add_attributes.ll`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150156

Files:
  clang/test/OpenMP/interop_irbuilder.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/test/Transforms/OpenMP/add_attributes.ll
  openmp/libomptarget/src/interop.cpp

Index: openmp/libomptarget/src/interop.cpp
===
--- openmp/libomptarget/src/interop.cpp
+++ openmp/libomptarget/src/interop.cpp
@@ -184,7 +184,7 @@
 void __tgt_interop_init(ident_t *LocRef, kmp_int32 Gtid,
 omp_interop_val_t *,
 kmp_interop_type_t InteropType, kmp_int32 DeviceId,
-kmp_int64 Ndeps, kmp_depend_info_t *DepList,
+kmp_int32 Ndeps, kmp_depend_info_t *DepList,
 kmp_int32 HaveNowait) {
   kmp_int32 NdepsNoalias = 0;
   kmp_depend_info_t *NoaliasDepList = NULL;
Index: llvm/test/Transforms/OpenMP/add_attributes.ll
===
--- llvm/test/Transforms/OpenMP/add_attributes.ll
+++ llvm/test/Transforms/OpenMP/add_attributes.ll
@@ -742,7 +742,7 @@
 
 declare void @__tgt_interop_destroy(ptr, i32, ptr, i32, i32, ptr, i32);
 
-declare void @__tgt_interop_init(ptr, i32, ptr, i32, i32, i64, ptr, i32);
+declare void @__tgt_interop_init(ptr, i32, ptr, i32, i32, i32, ptr, i32);
 
 declare void @__tgt_interop_use(ptr, i32, ptr, i32, i32, ptr, i32);
 
@@ -1398,7 +1398,7 @@
 ; CHECK: declare void @__tgt_interop_destroy(ptr, i32, ptr, i32, i32, ptr, i32)
 
 ; CHECK-NOT: Function Attrs
-; CHECK: declare void @__tgt_interop_init(ptr, i32, ptr, i32, i32, i64, ptr, i32)
+; CHECK: declare void @__tgt_interop_init(ptr, i32, ptr, i32, i32, i32, ptr, i32)
 
 ; CHECK-NOT: Function Attrs
 ; CHECK: declare void @__tgt_interop_use(ptr, i32, ptr, i32, i32, ptr, i32)
@@ -2046,7 +2046,7 @@
 ; OPTIMISTIC: declare void @__tgt_interop_destroy(ptr, i32, ptr, i32, i32, ptr, i32)
 
 ; OPTIMISTIC-NOT: Function Attrs
-; OPTIMISTIC: declare void @__tgt_interop_init(ptr, i32, ptr, i32, i32, i64, ptr, i32)
+; OPTIMISTIC: declare void @__tgt_interop_init(ptr, i32, ptr, i32, i32, i32, ptr, i32)
 
 ; OPTIMISTIC-NOT: Function Attrs
 ; OPTIMISTIC: declare void @__tgt_interop_use(ptr, i32, ptr, i32, i32, ptr, i32)
@@ -2707,7 +2707,7 @@
 ; EXT: declare void @__tgt_interop_destroy(ptr, i32 signext, ptr, i32 signext, i32 signext, ptr, i32 signext)
 
 ; EXT-NOT: Function Attrs
-; EXT: declare void @__tgt_interop_init(ptr, i32 signext, ptr, i32 signext, i32 signext, i64, ptr, i32 signext)
+; EXT: declare void @__tgt_interop_init(ptr, i32 signext, ptr, i32 signext, i32 signext, i32, ptr, i32 signext)
 
 ; EXT-NOT: Function Attrs
 ; EXT: declare void @__tgt_interop_use(ptr, i32 signext, ptr, i32 signext, i32 signext, ptr, i32 signext)
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -3798,7 +3798,7 @@
 Device = ConstantInt::get(Int32, -1);
   Constant *InteropTypeVal = ConstantInt::get(Int32, (int)InteropType);
   if (NumDependences == nullptr) {
-NumDependences = ConstantInt::get(Int64, 0);
+NumDependences = ConstantInt::get(Int32, 0);
 PointerType *PointerTypeVar = Type::getInt8PtrTy(M.getContext());
 DependenceAddress = ConstantPointerNull::get(PointerTypeVar);
   }
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -402,7 +402,7 @@
 __OMP_RTL(__kmpc_free, false, Void, /* Int */ Int32, VoidPtr, VoidPtr)
 
 __OMP_RTL(__tgt_interop_init, false, Void, IdentPtr, Int32, VoidPtrPtr, Int32,
-  Int32, Int64, VoidPtr, Int32)
+  Int32, Int32, VoidPtr, Int32)
 __OMP_RTL(__tgt_interop_destroy, false, Void, IdentPtr, Int32, VoidPtrPtr,
   Int32, Int32, VoidPtr, Int32)
 __OMP_RTL(__tgt_interop_use, false, Void, IdentPtr, Int32, VoidPtrPtr, Int32,
Index: clang/test/OpenMP/interop_irbuilder.cpp
===
--- clang/test/OpenMP/interop_irbuilder.cpp
+++ clang/test/OpenMP/interop_irbuilder.cpp
@@ -10,23 +10,17 @@
   int D0, D1;
   omp_interop_t interop;
 
-#pragma omp interop init(target \
- : interop)
+#pragma omp interop init(target : interop)
 
-#pragma omp interop init(targetsync \
- : interop)
+#pragma omp interop init(targetsync : interop)
 
-#pragma omp interop init(target \
- : interop) device(device_id)
+#pragma omp interop init(target : interop) device(device_id)

[PATCH] D150156: [OpenMP] Fix incorrect interop type for number of dependencies

2023-05-08 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D150156#4328360 , @tianshilei1992 
wrote:

> Does this cause the IR issue?

Not sure, I just get an undefined symbol error in the linker now. Not sure if 
that means it's resolved or I just can't reproduce it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150156

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


[PATCH] D150156: [OpenMP] Fix incorrect interop type for number of dependencies

2023-05-08 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 accepted this revision.
tianshilei1992 added a comment.
This revision is now accepted and ready to land.

Does this cause the IR issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150156

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


[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

2023-05-08 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:109-113
+Warnings
+
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15 (fixes 
`#62353`_,
+  fallout from the non-POD packing ABI fix in LLVM 15)

dblaikie wrote:
> aaron.ballman wrote:
> > I think this should move down to `Improvements to Clang's diagnostics` 
> > instead of adding a new category for it.
> Ah, hadn't spotted that category - thanks!
Failed to include the fix there in the initial commit, but got it in 793c5b1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149182

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


[PATCH] D148851: Disable llvm-symbolizer on some of the driver tests that are timing out

2023-05-08 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment.

In D148851#4328084 , @dblaikie wrote:

> In D148851#4311266 , @ahatanak 
> wrote:
>
>> Disable llvm-symbolizer when running lit tests.
>
> This seems problematic though - when lit tests fail it's quite helpful to get 
> a symbolized stack trace.

While I have seen this again just today with the driver tests and it is 
annoying, I would also prefer a more targeted solution if possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148851

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


[clang] 793c5b1 - Fix for release notes (follow-up to D149182/a8b0c6fa)

2023-05-08 Thread David Blaikie via cfe-commits

Author: David Blaikie
Date: 2023-05-09T00:40:11Z
New Revision: 793c5b12b9a70e363be40c5da2e26d7151fbbf41

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

LOG: Fix for release notes (follow-up to D149182/a8b0c6fa)

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index aa9a8a60e586..fcc6fb7df0c8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -111,12 +111,6 @@ Resolutions to C++ Defect Reports
 - Implemented `DR2397 `_ which allows ``auto`` 
specifier for pointers
   and reference to arrays.
 
-Warnings
-
-- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
-  Clang ABI >= 15 (fixes 
`#62353`_,
-  fallout from the non-POD packing ABI fix in LLVM 15)
-
 C Language Changes
 --
 - Support for outputs from asm goto statements along indirect edges has been
@@ -267,6 +261,11 @@ Improvements to Clang's diagnostics
   (`#62247: `_).
 - Clang now diagnoses shadowing of lambda's template parameter by a capture.
   (`#61105: `_).
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15.
+  (`#62353: `_,
+  fallout from the non-POD packing ABI fix in LLVM 15).
+
 
 Bug Fixes in This Version
 -



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


[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

2023-05-08 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:109-113
+Warnings
+
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15 (fixes 
`#62353`_,
+  fallout from the non-POD packing ABI fix in LLVM 15)

aaron.ballman wrote:
> I think this should move down to `Improvements to Clang's diagnostics` 
> instead of adding a new category for it.
Ah, hadn't spotted that category - thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149182

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


[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

2023-05-08 Thread David Blaikie via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
dblaikie marked an inline comment as done.
Closed by commit rGa8b0c6fa28ac: Remove -Wpacked false positive for non-pod 
types where the layout isnt… (authored by dblaikie).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149182

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/CodeGenCXX/warn-padded-packed.cpp
  clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp

Index: clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
===
--- clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
+++ clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
@@ -6,6 +6,8 @@
 // RUN: -fdump-record-layouts -fsyntax-only -verify -x c++ < %s | \
 // RUN:   FileCheck %s
 
+// expected-no-diagnostics
+
 struct A {
   double d;
 };
@@ -14,7 +16,7 @@
   char x[8];
 };
 
-struct [[gnu::packed]] C : B, A { // expected-warning{{packed attribute is unnecessary for 'C'}}
+struct [[gnu::packed]] C : B, A {
   char x alignas(4)[8];
 };
 
Index: clang/test/CodeGenCXX/warn-padded-packed.cpp
===
--- clang/test/CodeGenCXX/warn-padded-packed.cpp
+++ clang/test/CodeGenCXX/warn-padded-packed.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify %s -emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify=expected,top %s -emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify=expected,abi15 -fclang-abi-compat=15 %s -emit-llvm-only
 
 struct S1 {
   char c;
@@ -154,7 +155,7 @@
   char c1;
   short s1;
   char c2;
-  S28_non_pod p1; // expected-warning {{not packing field 'p1' as it is non-POD for the purposes of layout}}
+  S28_non_pod p1; // top-warning {{not packing field 'p1' as it is non-POD for the purposes of layout}}
 } __attribute__((packed));
 
 struct S29_non_pod_align_1 {
@@ -167,6 +168,16 @@
 } __attribute__((packed)); // no warning
 static_assert(alignof(S29) == 1, "");
 
+struct S30 {
+protected:
+ short s;
+} __attribute__((packed)); // no warning
+struct S30_use { // abi15-warning {{packed attribute is unnecessary for 'S30_use'}}
+  char c;
+  S30 u;
+} __attribute__((packed));
+static_assert(sizeof(S30_use) == 3, "");
+
 // The warnings are emitted when the layout of the structs is computed, so we have to use them.
 void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*,
S14*, S15*, S16*, S17*, S18*, S19*, S20*, S21*, S22*, S23*, S24*, S25*,
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2198,11 +2198,19 @@
   << (InBits ? 1 : 0); // (byte|bit)
 }
 
+const auto *CXXRD = dyn_cast(RD);
+
 // Warn if we packed it unnecessarily, when the unpacked alignment is not
 // greater than the one after packing, the size in bits doesn't change and
 // the offset of each field is identical.
+// Unless the type is non-POD (for Clang ABI > 15), where the packed
+// attribute on such a type does allow the type to be packed into other
+// structures that use the packed attribute.
 if (Packed && UnpackedAlignment <= Alignment &&
-UnpackedSizeInBits == getSizeInBits() && !HasPackedField)
+UnpackedSizeInBits == getSizeInBits() && !HasPackedField &&
+(!CXXRD || CXXRD->isPOD() ||
+ Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver15))
   Diag(D->getLocation(), diag::warn_unnecessary_packed)
   << Context.getTypeDeclType(RD);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -111,6 +111,12 @@
 - Implemented `DR2397 `_ which allows ``auto`` specifier for pointers
   and reference to arrays.
 
+Warnings
+
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15 (fixes `#62353`_,
+  fallout from the non-POD packing ABI fix in LLVM 15)
+
 C Language Changes
 --
 - Support for outputs from asm goto statements along indirect edges has been
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] a8b0c6f - Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

2023-05-08 Thread David Blaikie via cfe-commits

Author: David Blaikie
Date: 2023-05-09T00:13:45Z
New Revision: a8b0c6fa28acced71db33e80bd0b51d00422035b

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

LOG: Remove -Wpacked false positive for non-pod types where the layout isn't 
directly changed

The packed attribute can still be useful in this case if the struct is
then placed inside another packed struct - the non-pod element type's
packed attribute declares that it's OK to misalign this element inside
the packed structure. (otherwise the non-pod element is not packed/its
alignment is preserved, as per D117616/2771233)

Fixes PR62353

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/AST/RecordLayoutBuilder.cpp
clang/test/CodeGenCXX/warn-padded-packed.cpp
clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4f8165bbc8ee5..aa9a8a60e586d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -111,6 +111,12 @@ Resolutions to C++ Defect Reports
 - Implemented `DR2397 `_ which allows ``auto`` 
specifier for pointers
   and reference to arrays.
 
+Warnings
+
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15 (fixes 
`#62353`_,
+  fallout from the non-POD packing ABI fix in LLVM 15)
+
 C Language Changes
 --
 - Support for outputs from asm goto statements along indirect edges has been

diff  --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 641e9d62149e9..aca50912dceac 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2198,11 +2198,19 @@ void ItaniumRecordLayoutBuilder::FinishLayout(const 
NamedDecl *D) {
   << (InBits ? 1 : 0); // (byte|bit)
 }
 
+const auto *CXXRD = dyn_cast(RD);
+
 // Warn if we packed it unnecessarily, when the unpacked alignment is not
 // greater than the one after packing, the size in bits doesn't change and
 // the offset of each field is identical.
+// Unless the type is non-POD (for Clang ABI > 15), where the packed
+// attribute on such a type does allow the type to be packed into other
+// structures that use the packed attribute.
 if (Packed && UnpackedAlignment <= Alignment &&
-UnpackedSizeInBits == getSizeInBits() && !HasPackedField)
+UnpackedSizeInBits == getSizeInBits() && !HasPackedField &&
+(!CXXRD || CXXRD->isPOD() ||
+ Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver15))
   Diag(D->getLocation(), diag::warn_unnecessary_packed)
   << Context.getTypeDeclType(RD);
   }

diff  --git a/clang/test/CodeGenCXX/warn-padded-packed.cpp 
b/clang/test/CodeGenCXX/warn-padded-packed.cpp
index cf4890e40005d..c51a6c9443f6e 100644
--- a/clang/test/CodeGenCXX/warn-padded-packed.cpp
+++ b/clang/test/CodeGenCXX/warn-padded-packed.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify %s 
-emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked 
-verify=expected,top %s -emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked 
-verify=expected,abi15 -fclang-abi-compat=15 %s -emit-llvm-only
 
 struct S1 {
   char c;
@@ -154,7 +155,7 @@ struct S28 {
   char c1;
   short s1;
   char c2;
-  S28_non_pod p1; // expected-warning {{not packing field 'p1' as it is 
non-POD for the purposes of layout}}
+  S28_non_pod p1; // top-warning {{not packing field 'p1' as it is non-POD for 
the purposes of layout}}
 } __attribute__((packed));
 
 struct S29_non_pod_align_1 {
@@ -167,6 +168,16 @@ struct S29 {
 } __attribute__((packed)); // no warning
 static_assert(alignof(S29) == 1, "");
 
+struct S30 {
+protected:
+ short s;
+} __attribute__((packed)); // no warning
+struct S30_use { // abi15-warning {{packed attribute is unnecessary for 
'S30_use'}}
+  char c;
+  S30 u;
+} __attribute__((packed));
+static_assert(sizeof(S30_use) == 3, "");
+
 // The warnings are emitted when the layout of the structs is computed, so we 
have to use them.
 void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*,
S14*, S15*, S16*, S17*, S18*, S19*, S20*, S21*, S22*, S23*, S24*, S25*,

diff  --git a/clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp 
b/clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
index 37982fcb74b96..1576b8561cb56 100644
--- a/clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
+++ b/clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
@@ -6,6 +6,8 @@
 

[PATCH] D150156: [OpenMP] Fix incorrect interop type for number of dependencies

2023-05-08 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 520526.
jhuber6 added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150156

Files:
  clang/test/OpenMP/interop_irbuilder.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  openmp/libomptarget/src/interop.cpp

Index: openmp/libomptarget/src/interop.cpp
===
--- openmp/libomptarget/src/interop.cpp
+++ openmp/libomptarget/src/interop.cpp
@@ -184,7 +184,7 @@
 void __tgt_interop_init(ident_t *LocRef, kmp_int32 Gtid,
 omp_interop_val_t *,
 kmp_interop_type_t InteropType, kmp_int32 DeviceId,
-kmp_int64 Ndeps, kmp_depend_info_t *DepList,
+kmp_int32 Ndeps, kmp_depend_info_t *DepList,
 kmp_int32 HaveNowait) {
   kmp_int32 NdepsNoalias = 0;
   kmp_depend_info_t *NoaliasDepList = NULL;
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -3798,7 +3798,7 @@
 Device = ConstantInt::get(Int32, -1);
   Constant *InteropTypeVal = ConstantInt::get(Int32, (int)InteropType);
   if (NumDependences == nullptr) {
-NumDependences = ConstantInt::get(Int64, 0);
+NumDependences = ConstantInt::get(Int32, 0);
 PointerType *PointerTypeVar = Type::getInt8PtrTy(M.getContext());
 DependenceAddress = ConstantPointerNull::get(PointerTypeVar);
   }
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -402,7 +402,7 @@
 __OMP_RTL(__kmpc_free, false, Void, /* Int */ Int32, VoidPtr, VoidPtr)
 
 __OMP_RTL(__tgt_interop_init, false, Void, IdentPtr, Int32, VoidPtrPtr, Int32,
-  Int32, Int64, VoidPtr, Int32)
+  Int32, Int32, VoidPtr, Int32)
 __OMP_RTL(__tgt_interop_destroy, false, Void, IdentPtr, Int32, VoidPtrPtr,
   Int32, Int32, VoidPtr, Int32)
 __OMP_RTL(__tgt_interop_use, false, Void, IdentPtr, Int32, VoidPtrPtr, Int32,
Index: clang/test/OpenMP/interop_irbuilder.cpp
===
--- clang/test/OpenMP/interop_irbuilder.cpp
+++ clang/test/OpenMP/interop_irbuilder.cpp
@@ -10,23 +10,17 @@
   int D0, D1;
   omp_interop_t interop;
 
-#pragma omp interop init(target \
- : interop)
+#pragma omp interop init(target : interop)
 
-#pragma omp interop init(targetsync \
- : interop)
+#pragma omp interop init(targetsync : interop)
 
-#pragma omp interop init(target \
- : interop) device(device_id)
+#pragma omp interop init(target : interop) device(device_id)
 
-#pragma omp interop init(targetsync \
- : interop) device(device_id)
+#pragma omp interop init(targetsync : interop) device(device_id)
 
-#pragma omp interop use(interop) depend(in \
-: D0, D1) nowait
+#pragma omp interop use(interop) depend(in : D0, D1) nowait
 
-#pragma omp interop destroy(interop) depend(in \
-: D0, D1)
+#pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 
 struct S {
@@ -39,23 +33,17 @@
   int device_id = 4;
   int D0, D1;
 
-#pragma omp interop init(target \
- : interop)
+#pragma omp interop init(target : interop)
 
-#pragma omp interop init(targetsync \
- : interop)
+#pragma omp interop init(targetsync : interop)
 
-#pragma omp interop init(target \
- : interop) device(device_id)
+#pragma omp interop init(target : interop) device(device_id)
 
-#pragma omp interop init(targetsync \
- : interop) device(device_id)
+#pragma omp interop init(targetsync : interop) device(device_id)
 
-#pragma omp interop use(interop) depend(in \
-: D0, D1) nowait
+#pragma omp interop use(interop) depend(in : D0, D1) nowait
 
-#pragma omp interop destroy(interop) depend(in \
-: D0, D1)
+#pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 // CHECK-LABEL: @_Z5test1v(
 // CHECK-NEXT:  entry:
@@ -69,15 +57,15 @@
 // CHECK-NEXT:[[DEP_COUNTER_ADDR6:%.*]] = alloca i64, align 8
 // CHECK-NEXT:store i32 4, ptr [[DEVICE_ID]], align 4
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1:[0-9]+]])
-// CHECK-NEXT:call void @__tgt_interop_init(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM]], ptr [[INTEROP]], i32 1, i32 -1, i64 

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

2023-05-08 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D141451#4312256 , @aaron.ballman 
wrote:

> In D141451#4311199 , @dblaikie 
> wrote:
>
>>> probably too much, but then I wonder "how do we make Fortify work?".
>>
>> (I'm still sort of on the side of "if the compile-time costs to get more 
>> verbose diagnostics is high, we could have a low-quality default-on 
>> diagnostic and it could mention/recommend a different/high 
>> quality/compile-time costly diagnostic" so for code bases like the Linux 
>> kernel that rely on this sort of thing a lot, with lots of always_inlining, 
>> etc, they can tradeoff toward the better diagnostic experience and codebases 
>> that don't use these features much they can still get the checking for when 
>> it does come up, but at some cost of quality/awkwardness in needing to 
>> rebuild with other flags)
>
> This isn't about verbosity of the diagnostics, though, so much as it's about 
> user experience. I'd agree that enabling more verbose diagnostic checking is 
> reasonable to ask users to opt into, but what I'm struggling with is 
> expecting users to opt into a mode so that the diagnostics appear in the 
> correct places instead of detached from the source they're noting. Also, with 
> the `error` and `warning` attributes both being used by glibc (and not just 
> for fortify source, but as a general part of the interface), I think this is 
> broader than just inlining decision notes.
>
>> But if folks decided some more invasive tracking was worth implementing 
>> (alongside the debug info codepath that already exists but is perhaps too 
>> slow to be always on) I wouldn't get up on a soapbox to strenuously object - 
>> I just think it's a bit unfortunate to build a duplicate thing, but maybe 
>> necessary.
>
> Agreed on the unfortunateness of duplicating functionality; if we can avoid 
> that, it'd be best. But I'm not sure we have more ideas on how to accomplish 
> it, either.

Fair enough. Well, hopefully somewhere in the summary of all this is a perf 
comparison - between the most narrowed down option using the existing DebugLocs 
and this new/distinct tracking system. But guess it's going that way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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


[PATCH] D148851: Disable llvm-symbolizer on some of the driver tests that are timing out

2023-05-08 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D148851#4311266 , @ahatanak wrote:

> Disable llvm-symbolizer when running lit tests.

This seems problematic though - when lit tests fail it's quite helpful to get a 
symbolized stack trace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148851

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


[PATCH] D149917: [lld][WebAssembly] Add --preserve-features flag

2023-05-08 Thread Derek Schuff via Phabricator via cfe-commits
dschuff added a comment.

or use `--keep-section` to match objcopy/strip? 
https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-objcopy/wasm/basic-keep.test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149917

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


[PATCH] D149917: [lld][WebAssembly] Add --preserve-features flag

2023-05-08 Thread Derek Schuff via Phabricator via cfe-commits
dschuff added a comment.

Yeah, I think that would work. Or maybe `--preserve-sections=sec1,sec2` since 
we might want to preserve multiple sections.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149917

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


[PATCH] D150151: [clang] Prevent creation of new submodules in ASTWriter

2023-05-08 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir added a comment.

No test, because I haven't come up with a test case that wouldn't be 
invalidated by https://reviews.llvm.org/D103930. Let me know if you have any 
ideas.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150151

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


[PATCH] D150151: [clang] Prevent creation of new submodules in ASTWriter

2023-05-08 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir created this revision.
benlangmuir added a reviewer: jansvoboda11.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Avoid inferring new submodules for headers in ASTWriter's collection of 
affecting modulemap files, since we don't want to pick up dependencies that 
didn't actually exist during parsing.

rdar://108681805


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150151

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Serialization/ASTWriter.cpp


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -185,7 +185,8 @@
 if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
   continue;
 
-for (const auto  : HS.findAllModulesForHeader(File)) {
+for (const auto  :
+ HS.findAllModulesForHeader(File, /*AllowCreation=*/false)) {
   if (!KH.getModule())
 continue;
   ModulesToProcess.push_back(KH.getModule());
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -683,12 +683,12 @@
 }
 
 ArrayRef
-ModuleMap::findAllModulesForHeader(const FileEntry *File) {
+ModuleMap::findAllModulesForHeader(const FileEntry *File, bool AllowCreation) {
   HeadersMap::iterator Known = findKnownHeader(File);
   if (Known != Headers.end())
 return Known->second;
 
-  if (findOrCreateModuleForHeaderInUmbrellaDir(File))
+  if (AllowCreation && findOrCreateModuleForHeaderInUmbrellaDir(File))
 return Headers.find(File)->second;
 
   return std::nullopt;
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1565,13 +1565,14 @@
 }
 
 ArrayRef
-HeaderSearch::findAllModulesForHeader(const FileEntry *File) const {
+HeaderSearch::findAllModulesForHeader(const FileEntry *File,
+  bool AllowCreation) const {
   if (ExternalSource) {
 // Make sure the external source has handled header info about this file,
 // which includes whether the file is part of a module.
 (void)getExistingFileInfo(File);
   }
-  return ModMap.findAllModulesForHeader(File);
+  return ModMap.findAllModulesForHeader(File, AllowCreation);
 }
 
 static bool suggestModule(HeaderSearch , const FileEntry *File,
Index: clang/include/clang/Lex/ModuleMap.h
===
--- clang/include/clang/Lex/ModuleMap.h
+++ clang/include/clang/Lex/ModuleMap.h
@@ -448,9 +448,13 @@
   /// and does not consult the external source. (Those checks are the
   /// responsibility of \ref HeaderSearch.)
   ///
+  /// \param AllowCreation Whether to allow inference of a new submodule, or to
+  ///only return existing known modules.
+  ///
   /// Typically, \ref findModuleForHeader should be used instead, as it picks
   /// the preferred module for the header.
-  ArrayRef findAllModulesForHeader(const FileEntry *File);
+  ArrayRef findAllModulesForHeader(const FileEntry *File,
+bool AllowCreation = true);
 
   /// Like \ref findAllModulesForHeader, but do not attempt to infer module
   /// ownership from umbrella headers if we've not already done so.
Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -665,9 +665,13 @@
 
   /// Retrieve all the modules corresponding to the given file.
   ///
+  /// \param AllowCreation Whether to allow inference of a new submodule, or to
+  ///only return existing known modules.
+  ///
   /// \ref findModuleForHeader should typically be used instead of this.
   ArrayRef
-  findAllModulesForHeader(const FileEntry *File) const;
+  findAllModulesForHeader(const FileEntry *File,
+  bool AllowCreation = true) const;
 
   /// Read the contents of the given module map file.
   ///


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -185,7 +185,8 @@
 if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
   continue;
 
-for (const auto  : HS.findAllModulesForHeader(File)) {
+for (const auto  :
+ HS.findAllModulesForHeader(File, /*AllowCreation=*/false)) {
   if (!KH.getModule())
 continue;
   ModulesToProcess.push_back(KH.getModule());
Index: 

[PATCH] D146399: [AIX][Clang][K] Create `-K` Option for AIX.

2023-05-08 Thread Michael Francis 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 rG5da7f30f24c4: [AIX][Clang][K] Create `-K` Option for AIX. 
(authored by francii).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146399

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/aix-ld.c
  clang/test/Driver/unsupported-target-K.c


Index: clang/test/Driver/unsupported-target-K.c
===
--- /dev/null
+++ clang/test/Driver/unsupported-target-K.c
@@ -0,0 +1,8 @@
+// Check powerpc64-unknown-linux-gnu. -K not supported.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc64-unknown-linux-gnu \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck --check-prefixes=CHECK-K-SUPPORT %s
+// CHECK-K-SUPPORT: clang: error: unsupported option '-K' for target 
'powerpc64-unknown-linux-gnu'
Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -1096,3 +1096,27 @@
 // CHECK-RELOCATABLE-NOT: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-RELOCATABLE-NOT: "-l{{.*}}"
 // CHECK-RELOCATABLE-NOT: "-L{{.*}}"
+
+// Check powerpc-ibm-aix7.1.0.0. -K is a passthrough linker option.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck --check-prefixes=CHECK-K %s
+// CHECK-K: "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-K: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-K: "{{.*}}ld{{(.exe)?}}"
+// CHECK-K: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-K: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-K: "-K"
+
+// Check powerpc-ibm-aix7.1.0.0. -K unused when not linking.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:-c \
+// RUN:   | FileCheck --check-prefixes=CHECK-K-UNUSED %s
+// CHECK-K-UNUSED: clang: warning: -K: 'linker' input unused 
[-Wunused-command-line-argument]
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6326,6 +6326,11 @@
 }
   }
 
+  if (Arg *A = Args.getLastArgNoClaim(options::OPT_K);
+  A && !TC.getTriple().isOSAIX())
+D.Diag(diag::err_drv_unsupported_opt_for_target)
+<< A->getAsString(Args) << TripleStr;
+
   if (Args.getLastArg(options::OPT_fapple_kext) ||
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
 CmdArgs.push_back("-fapple-kext");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3421,6 +3421,7 @@
   HelpText<"Overlay the virtual filesystem described by file over the real 
file system. "
"Additionally, pass this overlay file to the linker if it supports 
it">;
 def imultilib : Separate<["-"], "imultilib">, Group;
+def K : Flag<["-"], "K">, Flags<[LinkerInput]>;
 def keep__private__externs : Flag<["-"], "keep_private_externs">;
 def l : JoinedOrSeparate<["-"], "l">, Flags<[LinkerInput, RenderJoined]>,
 Group;


Index: clang/test/Driver/unsupported-target-K.c
===
--- /dev/null
+++ clang/test/Driver/unsupported-target-K.c
@@ -0,0 +1,8 @@
+// Check powerpc64-unknown-linux-gnu. -K not supported.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc64-unknown-linux-gnu \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck --check-prefixes=CHECK-K-SUPPORT %s
+// CHECK-K-SUPPORT: clang: error: unsupported option '-K' for target 'powerpc64-unknown-linux-gnu'
Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -1096,3 +1096,27 @@
 // CHECK-RELOCATABLE-NOT: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-RELOCATABLE-NOT: "-l{{.*}}"
 // CHECK-RELOCATABLE-NOT: "-L{{.*}}"
+
+// Check powerpc-ibm-aix7.1.0.0. -K is a passthrough linker option.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck 

[clang] 5da7f30 - [AIX][Clang][K] Create `-K` Option for AIX.

2023-05-08 Thread Michael Francis via cfe-commits

Author: Michael Francis
Date: 2023-05-08T22:53:44Z
New Revision: 5da7f30f24c4620c4f4425206fbdd0921d333dc0

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

LOG: [AIX][Clang][K] Create `-K` Option for AIX.

`-K` is a linker option on AIX, that is used to align the header, text, data, 
and loader sections of the output file so that each section begins on a page 
boundary.

This patch creates the `-K` option in clang. On non-AIX targets, the 
"unsupported option" error is thrown.

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

Added: 
clang/test/Driver/unsupported-target-K.c

Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/aix-ld.c

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 182f0290736d8..496264b74d460 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3421,6 +3421,7 @@ def vfsoverlay : JoinedOrSeparate<["-", "--"], 
"vfsoverlay">, Flags<[CC1Option,
   HelpText<"Overlay the virtual filesystem described by file over the real 
file system. "
"Additionally, pass this overlay file to the linker if it supports 
it">;
 def imultilib : Separate<["-"], "imultilib">, Group;
+def K : Flag<["-"], "K">, Flags<[LinkerInput]>;
 def keep__private__externs : Flag<["-"], "keep_private_externs">;
 def l : JoinedOrSeparate<["-"], "l">, Flags<[LinkerInput, RenderJoined]>,
 Group;

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 32ba56066af58..c12a6ab88097b 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6326,6 +6326,11 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 }
   }
 
+  if (Arg *A = Args.getLastArgNoClaim(options::OPT_K);
+  A && !TC.getTriple().isOSAIX())
+D.Diag(diag::err_drv_unsupported_opt_for_target)
+<< A->getAsString(Args) << TripleStr;
+
   if (Args.getLastArg(options::OPT_fapple_kext) ||
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
 CmdArgs.push_back("-fapple-kext");

diff  --git a/clang/test/Driver/aix-ld.c b/clang/test/Driver/aix-ld.c
index eb2910db239ff..d5c595495976a 100644
--- a/clang/test/Driver/aix-ld.c
+++ b/clang/test/Driver/aix-ld.c
@@ -1096,3 +1096,27 @@
 // CHECK-RELOCATABLE-NOT: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-RELOCATABLE-NOT: "-l{{.*}}"
 // CHECK-RELOCATABLE-NOT: "-L{{.*}}"
+
+// Check powerpc-ibm-aix7.1.0.0. -K is a passthrough linker option.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck --check-prefixes=CHECK-K %s
+// CHECK-K: "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-K: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-K: "{{.*}}ld{{(.exe)?}}"
+// CHECK-K: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-K: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-K: "-K"
+
+// Check powerpc-ibm-aix7.1.0.0. -K unused when not linking.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:-c \
+// RUN:   | FileCheck --check-prefixes=CHECK-K-UNUSED %s
+// CHECK-K-UNUSED: clang: warning: -K: 'linker' input unused 
[-Wunused-command-line-argument]

diff  --git a/clang/test/Driver/unsupported-target-K.c 
b/clang/test/Driver/unsupported-target-K.c
new file mode 100644
index 0..8b9a6f529c326
--- /dev/null
+++ b/clang/test/Driver/unsupported-target-K.c
@@ -0,0 +1,8 @@
+// Check powerpc64-unknown-linux-gnu. -K not supported.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc64-unknown-linux-gnu \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck --check-prefixes=CHECK-K-SUPPORT %s
+// CHECK-K-SUPPORT: clang: error: unsupported option '-K' for target 
'powerpc64-unknown-linux-gnu'



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


[PATCH] D146399: [AIX][Clang][K] Create `-K` Option for AIX.

2023-05-08 Thread Michael Francis via Phabricator via cfe-commits
francii updated this revision to Diff 520502.
francii added a comment.

Remove leftover marker


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146399

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/aix-ld.c
  clang/test/Driver/unsupported-target-K.c


Index: clang/test/Driver/unsupported-target-K.c
===
--- /dev/null
+++ clang/test/Driver/unsupported-target-K.c
@@ -0,0 +1,8 @@
+// Check powerpc64-unknown-linux-gnu. -K not supported.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc64-unknown-linux-gnu \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck --check-prefixes=CHECK-K-SUPPORT %s
+// CHECK-K-SUPPORT: clang: error: unsupported option '-K' for target 
'powerpc64-unknown-linux-gnu'
Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -1096,3 +1096,27 @@
 // CHECK-RELOCATABLE-NOT: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-RELOCATABLE-NOT: "-l{{.*}}"
 // CHECK-RELOCATABLE-NOT: "-L{{.*}}"
+
+// Check powerpc-ibm-aix7.1.0.0. -K is a passthrough linker option.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck --check-prefixes=CHECK-K %s
+// CHECK-K: "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-K: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-K: "{{.*}}ld{{(.exe)?}}"
+// CHECK-K: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-K: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-K: "-K"
+
+// Check powerpc-ibm-aix7.1.0.0. -K unused when not linking.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:-c \
+// RUN:   | FileCheck --check-prefixes=CHECK-K-UNUSED %s
+// CHECK-K-UNUSED: clang: warning: -K: 'linker' input unused 
[-Wunused-command-line-argument]
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6326,6 +6326,11 @@
 }
   }
 
+  if (Arg *A = Args.getLastArgNoClaim(options::OPT_K);
+  A && !TC.getTriple().isOSAIX())
+D.Diag(diag::err_drv_unsupported_opt_for_target)
+<< A->getAsString(Args) << TripleStr;
+
   if (Args.getLastArg(options::OPT_fapple_kext) ||
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
 CmdArgs.push_back("-fapple-kext");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3421,6 +3421,7 @@
   HelpText<"Overlay the virtual filesystem described by file over the real 
file system. "
"Additionally, pass this overlay file to the linker if it supports 
it">;
 def imultilib : Separate<["-"], "imultilib">, Group;
+def K : Flag<["-"], "K">, Flags<[LinkerInput]>;
 def keep__private__externs : Flag<["-"], "keep_private_externs">;
 def l : JoinedOrSeparate<["-"], "l">, Flags<[LinkerInput, RenderJoined]>,
 Group;


Index: clang/test/Driver/unsupported-target-K.c
===
--- /dev/null
+++ clang/test/Driver/unsupported-target-K.c
@@ -0,0 +1,8 @@
+// Check powerpc64-unknown-linux-gnu. -K not supported.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc64-unknown-linux-gnu \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck --check-prefixes=CHECK-K-SUPPORT %s
+// CHECK-K-SUPPORT: clang: error: unsupported option '-K' for target 'powerpc64-unknown-linux-gnu'
Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -1096,3 +1096,27 @@
 // CHECK-RELOCATABLE-NOT: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-RELOCATABLE-NOT: "-l{{.*}}"
 // CHECK-RELOCATABLE-NOT: "-L{{.*}}"
+
+// Check powerpc-ibm-aix7.1.0.0. -K is a passthrough linker option.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck --check-prefixes=CHECK-K %s
+// CHECK-K: "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-K: "-isysroot" "[[SYSROOT:[^"]+]]"
+// 

[PATCH] D149612: [Sema] avoid merge error type

2023-05-08 Thread Congcong Cai via Phabricator via cfe-commits
HerrCai0907 updated this revision to Diff 520496.
HerrCai0907 marked 2 inline comments as done.
HerrCai0907 added a comment.

use 1 replace 0 as length


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149612

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-dump-types-errors-json.cpp
  clang/test/AST/ast-dump-types-errors.cpp
  clang/test/Sema/merge-decls.c

Index: clang/test/Sema/merge-decls.c
===
--- clang/test/Sema/merge-decls.c
+++ clang/test/Sema/merge-decls.c
@@ -91,3 +91,7 @@
   int x[5];
   test7_f(); // expected-warning {{incompatible pointer types passing 'int (*)[5]' to parameter of type 'int (*)[10]}}
 }
+
+char d;
+char x[sizeof(d.data) == 8]; // expected-error {{member reference base type 'char' is not a structure or union}}
+char x[sizeof(d.data) == 4]; // expected-error {{member reference base type 'char' is not a structure or union}}
Index: clang/test/AST/ast-dump-types-errors.cpp
===
--- clang/test/AST/ast-dump-types-errors.cpp
+++ clang/test/AST/ast-dump-types-errors.cpp
@@ -2,5 +2,5 @@
 
 void test() {
   using ContainsErrors = int[sizeof(undef())];
-  // CHECK: DependentSizedArrayType {{.*}} contains-errors dependent
+  // CHECK: ConstantArrayType {{.*}} 'int[1]' contains-errors dependent 1
 }
Index: clang/test/AST/ast-dump-types-errors-json.cpp
===
--- clang/test/AST/ast-dump-types-errors-json.cpp
+++ clang/test/AST/ast-dump-types-errors-json.cpp
@@ -24,18 +24,19 @@
 // CHECK-NEXT:  },
 // CHECK-NEXT:  "name": "TestContainsErrors",
 // CHECK-NEXT:  "type": {
-// CHECK-NEXT:   "qualType": "int[sizeof ((undef))]"
+// CHECK-NEXT:   "qualType": "int[1]"
 // CHECK-NEXT:  },
 // CHECK-NEXT:  "inner": [
 // CHECK-NEXT:   {
 // CHECK-NEXT:"id": "0x{{.*}}",
-// CHECK-NEXT:"kind": "DependentSizedArrayType",
+// CHECK-NEXT:"kind": "ConstantArrayType",
 // CHECK-NEXT:"type": {
-// CHECK-NEXT: "qualType": "int[sizeof ((undef))]"
+// CHECK-NEXT: "qualType": "int[1]"
 // CHECK-NEXT:},
 // CHECK-NEXT:"containsErrors": true,
 // CHECK-NEXT:"isDependent": true,
 // CHECK-NEXT:"isInstantiationDependent": true,
+// CHECK-NEXT:"size": 1,
 // CHECK-NEXT:"inner": [
 // CHECK-NEXT: {
 // CHECK-NEXT:  "id": "0x{{.*}}",
@@ -43,96 +44,6 @@
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "qualType": "int"
 // CHECK-NEXT:  }
-// CHECK-NEXT: },
-// CHECK-NEXT: {
-// CHECK-NEXT:  "id": "0x{{.*}}",
-// CHECK-NEXT:  "kind": "UnaryExprOrTypeTraitExpr",
-// CHECK-NEXT:  "range": {
-// CHECK-NEXT:   "begin": {
-// CHECK-NEXT:"offset": {{[0-9]+}},
-// CHECK-NEXT:"col": 32,
-// CHECK-NEXT:"tokLen": 6
-// CHECK-NEXT:   },
-// CHECK-NEXT:   "end": {
-// CHECK-NEXT:"offset": {{[0-9]+}},
-// CHECK-NEXT:"col": 46,
-// CHECK-NEXT:"tokLen": 1
-// CHECK-NEXT:   }
-// CHECK-NEXT:  },
-// CHECK-NEXT:  "type": {
-// CHECK-NEXT:   "qualType": "unsigned long"
-// CHECK-NEXT:  },
-// CHECK-NEXT:  "valueCategory": "prvalue",
-// CHECK-NEXT:  "name": "sizeof",
-// CHECK-NEXT:  "inner": [
-// CHECK-NEXT:   {
-// CHECK-NEXT:"id": "0x{{.*}}",
-// CHECK-NEXT:"kind": "ParenExpr",
-// CHECK-NEXT:"range": {
-// CHECK-NEXT: "begin": {
-// CHECK-NEXT:  "offset": {{[0-9]+}},
-// CHECK-NEXT:  "col": 38,
-// CHECK-NEXT:  "tokLen": 1
-// CHECK-NEXT: },
-// CHECK-NEXT: "end": {
-// CHECK-NEXT:  "offset": {{[0-9]+}},
-// CHECK-NEXT:  "col": 46,
-// CHECK-NEXT:  "tokLen": 1
-// CHECK-NEXT: }
-// CHECK-NEXT:},
-// CHECK-NEXT:"type": {
-// CHECK-NEXT: "qualType": ""
-// CHECK-NEXT:},
-// CHECK-NEXT:"valueCategory": "lvalue",
-// CHECK-NEXT:"inner": [
-// CHECK-NEXT: {
-// CHECK-NEXT:  "id": "0x{{.*}}",
-// CHECK-NEXT:  "kind": "RecoveryExpr",
-// CHECK-NEXT:  "range": {
-// CHECK-NEXT:   "begin": {
-// CHECK-NEXT:"offset": {{[0-9]+}},
-// CHECK-NEXT:"col": 39,
-// CHECK-NEXT:"tokLen": 5
-// CHECK-NEXT:   },
-// CHECK-NEXT:   "end": {
-// CHECK-NEXT:"offset": {{[0-9]+}},
-// CHECK-NEXT:"col": 45,
-// CHECK-NEXT:"tokLen": 1
-// CHECK-NEXT:   }
-// CHECK-NEXT:  },
-// CHECK-NEXT:  "type": {
-// CHECK-NEXT:   "qualType": ""
-// CHECK-NEXT:  },
-// CHECK-NEXT:  "valueCategory": "lvalue",
-// CHECK-NEXT:  "inner": [
-// CHECK-NEXT:   {
-// CHECK-NEXT:"id": "0x{{.*}}",
-// CHECK-NEXT:"kind": 

[PATCH] D149612: [Sema] avoid merge error type

2023-05-08 Thread Congcong Cai via Phabricator via cfe-commits
HerrCai0907 added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:2583
+if (ArraySize->containsErrors()) {
+  RecoveryExpr *RE = RecoveryExpr::Create(
+  Context, ArraySize->getType(), ArraySize->getBeginLoc(),

erichkeane wrote:
> Actually thinking further... rather than create a NEW RecoveryExpr, is there 
> a problem KEEPING the ArraySize expression?  It'd likely give more 
> information/keep more AST consistency.
> 
> ALSO, creating this as size-0 has some implications we probably don't want.  
> I wonder if a different for the placeholder would be better?  
> 
> I'D ALSO suggest hoisting this ArraySize->containsError out of the else-if 
> and into its own branch (rather than inside the dependent checks).
> KEEPING the ArraySize expression
Agree.

> creating this as size-0 has some implications we probably don't want.
Do you think we can modify it as 1? But I don't find a better way to identifier 
it as undef or other.

> hoisting this ArraySize->containsError out of the else-if 
Done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149612

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


[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

2023-05-08 Thread Reid Kleckner via Phabricator via cfe-commits
rnk edited reviewers, added: hans, efriedma; removed: majnemer, rnk, aeubanks.
rnk added a comment.

Thanks for working on this, this is also an issue for our users. I've been out 
on leave. I replaced myself as a reviewer with Hans.


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

https://reviews.llvm.org/D137107

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


[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

2023-05-08 Thread NagaChaitanya Vellanki via Phabricator via cfe-commits
chaitanyav updated this revision to Diff 520491.
chaitanyav added a comment.

Rebase with upstream


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Analysis/uninit-vals.c
  clang/test/Sema/integer-overflow.c
  clang/test/Sema/parentheses.c
  clang/test/Sema/parentheses.cpp
  clang/test/SemaCXX/array-bounds.cpp
  clang/test/SemaCXX/integer-overflow.cpp
  libcxx/include/__chrono/duration.h
  libcxx/include/strstream
  libcxx/test/libcxx/algorithms/nth_element_stability.pass.cpp
  libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
  libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
  libcxxabi/src/cxa_personality.cpp

Index: libcxxabi/src/cxa_personality.cpp
===
--- libcxxabi/src/cxa_personality.cpp
+++ libcxxabi/src/cxa_personality.cpp
@@ -718,9 +718,7 @@
 if (actionEntry == 0)
 {
 // Found a cleanup
-results.reason = actions & _UA_SEARCH_PHASE
- ? _URC_CONTINUE_UNWIND
- : _URC_HANDLER_FOUND;
+results.reason = ((actions & _UA_SEARCH_PHASE) ? _URC_CONTINUE_UNWIND : _URC_HANDLER_FOUND);
 return;
 }
 // Convert 1-based byte offset into
@@ -832,9 +830,8 @@
 // End of action list. If this is phase 2 and we have found
 // a cleanup (ttypeIndex=0), return _URC_HANDLER_FOUND;
 // otherwise return _URC_CONTINUE_UNWIND.
-results.reason = hasCleanup && actions & _UA_CLEANUP_PHASE
- ? _URC_HANDLER_FOUND
- : _URC_CONTINUE_UNWIND;
+results.reason =
+(hasCleanup && (actions & _UA_CLEANUP_PHASE)) ? _URC_HANDLER_FOUND : _URC_CONTINUE_UNWIND;
 return;
 }
 // Go to next action
@@ -1243,10 +1240,9 @@
 {
 const __shim_type_info* excpType =
 static_cast(new_exception_header->exceptionType);
-adjustedPtr =
-__getExceptionClass(_exception_header->unwindHeader) == kOurDependentExceptionClass ?
-((__cxa_dependent_exception*)new_exception_header)->primaryException :
-new_exception_header + 1;
+adjustedPtr = (__getExceptionClass(_exception_header->unwindHeader) == kOurDependentExceptionClass)
+  ? ((__cxa_dependent_exception*)new_exception_header)->primaryException
+  : new_exception_header + 1;
 if (!exception_spec_can_catch(ttypeIndex, classInfo, ttypeEncoding,
   excpType, adjustedPtr,
   unwind_exception, base))
Index: libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
===
--- libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
+++ libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
@@ -32,7 +32,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   std::less comp;
   std::__sort_dispatch(v.begin(), v.end(), comp);
@@ -44,7 +44,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   auto deterministic_v = deterministic();
   std::sort(v.begin(), v.end());
@@ -62,7 +62,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   auto snapshot_v = v;
   auto snapshot_custom_v = v;
Index: libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
===
--- libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
+++ libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
@@ -32,7 +32,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = (i % 2 ? 1 : kSize / 2 + i);
+v[i].value = ((i % 2) ? 1 : kSize / 2 + i);
   }
   auto comp = std::less();
   std::__partial_sort_impl(v.begin(), v.begin() + kSize / 2, v.end(), comp);
@@ -44,7 +44,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = (i % 2 ? 1 : kSize / 2 + i);
+v[i].value = ((i % 2) ? 1 : kSize / 2 + i);
   }
   auto deterministic_v = deterministic();
  

[PATCH] D146399: [AIX][Clang][K] Create `-K` Option for AIX.

2023-05-08 Thread David Tenty via Phabricator via cfe-commits
daltenty accepted this revision.
daltenty added a comment.
This revision is now accepted and ready to land.

LGTM, with minor fixup as noted




Comment at: clang/test/Driver/aix-ld.c:1123
+// CHECK-K-UNUSED: clang: warning: -K: 'linker' input unused 
[-Wunused-command-line-argument]
+>>> 8f57d6ae6c1a ([AIX][Clang][K] Create `-K` Option for AIX.)

nit: I think you missed a conflict marker on the last update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146399

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


[PATCH] D149504: [clang][CodeGenPGO] Don't use an invalid index when region counts disagree

2023-05-08 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai accepted this revision.
smeenai added a comment.
This revision is now accepted and ready to land.

Seems pretty reasonable to add a bounds check here. You should probably add a 
comment explaining the reasoning though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149504

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


[PATCH] D149516: [Sema] `setInvalidDecl` for error deduction declaration

2023-05-08 Thread Congcong Cai via Phabricator via cfe-commits
HerrCai0907 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149516

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


[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

2023-05-08 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir added a comment.

We have several new build failures with this change that I'm looking through. 
So far, a common one is an error of the form

  /source/module.modulemap: error: redefinition of module
  /build/Foo.framework/Modules/module.modulemap: note: previously defined here

ie. we're now finding the same module in two places where we didn't before. 
It's possible we could avoid this for framework modules if we ignore framework 
modules that are not actually in framework directories, but we have seen this 
with non-frameworks modules as well.

Another class of errors is where the module we find does not compile in the 
current context, or it doesn't provide what the includer wanted when built as a 
module. Basically we silently depended on it being a textual include.

Still looking at issues and not sure whether these are blockers or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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


[PATCH] D149917: [lld][WebAssembly] Add --preserve-features flag

2023-05-08 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment.

In D149917#4322367 , @dschuff wrote:

> Do we want to make this any more general? In the future we might want to 
> preserve other sections, e.g. passing optimization or profiling info from 
> LLVM to Binaryen. Or maybe JSPI info?

Do you have something you suggest?   Something like 
`--preserve-section=custom_section_name`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149917

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


[PATCH] D139749: Headers: use C++ inline semantics in C++ mode

2023-05-08 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a subscriber: hans.
rnk added a comment.

This feels to me like we are still working around some incompatibilities 
between the MSVC intrin.h / intrin0.h model. I would prefer it if we could 
always use `static inline` consistently in our intrinsic headers so we don't 
have to worry about ODR violation problems. However, we're using always_inline 
so there's not much to worry about anyway.

Anyway, if it comes up again, we should try to come up with a better fix. +@hans


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139749

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


[PATCH] D149867: [M68k] Add Clang support for the new M68k_RTD CC

2023-05-08 Thread Min-Yih Hsu via Phabricator via cfe-commits
myhsu added a comment.

In D149867#4325040 , @jrtc27 wrote:

> So GCC gives me:
>
>   warning: ‘stdcall’ attribute directive ignored [-Wattributes]
>
> when trying to use `__attribute__((stdcall))` on m68k, which matches the fact 
> it's only mentioned in the manage for the x86 option.

In principal, I wanted to reuse as much existing code path as possible to 
handle `-mrtd`, primarily because these two calling conventions are basically 
identical and I didn't want to create too much churn. Reusing both 
`DCC_StdCall` and `CC_X86StdCall` serves that purpose. Making 
`__attribute__((stdcall))` available to m68k is just a side effect that I felt 
harmless.
I'm fine to create a separate `CC_M68kRTDCall` or even `DCC_RTDCall`. What do 
you think @jrtc27 ?

> Interestingly it seems to ICE during expand when I use -mrtd though...

I guess you're referring to GCC? I'm curious which snippet you used.


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

https://reviews.llvm.org/D149867

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


[PATCH] D146030: [clang][Interp] Handle LambdaExprs

2023-05-08 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/test/AST/Interp/lambda.cpp:5
+constexpr int a = 12;
+constexpr int f = [c = a]() { return c; }();
+static_assert(f == a);

Fun case

```
int constexpr f() {
return [x = 10] {
   decltype(x) y; // type int b/c not odr use
  // refers to original init-capture
   auto  = x; // type const int & b/c odr use
 // refers to lambdas copy of x
y = 10; // Ok
//z = 10; // Ill-formed
return y;
}();
}

constexpr int x = f();
```


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

https://reviews.llvm.org/D146030

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


[PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

2023-05-08 Thread Andrey Ali Khan Bolshakov via Phabricator via cfe-commits
bolshakov-a updated this revision to Diff 520478.
bolshakov-a added a subscriber: hubert.reinterpretcast.
bolshakov-a added a comment.

Avoid binding references in template arguments to bit-fields. @erichkeane, 
@hubert.reinterpretcast, please verify.


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

https://reviews.llvm.org/D140996

Files:
  clang-tools-extra/clangd/DumpAST.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ODRHash.h
  clang/include/clang/AST/PropertiesBase.td
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TemplateArgumentVisitor.h
  clang/include/clang/AST/TemplateBase.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/ODRHash.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CXX/drs/dr12xx.cpp
  clang/test/CXX/temp/temp.arg/temp.arg.nontype/p1.cpp
  clang/test/CodeGenCXX/mangle-ms-templates.cpp
  clang/test/CodeGenCXX/mangle-template.cpp
  clang/test/CodeGenCXX/template-arguments.cpp
  clang/test/Index/USR/structural-value-tpl-arg.cpp
  clang/test/Modules/odr_hash.cpp
  clang/test/SemaCXX/warn-bool-conversion.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  clang/www/cxx_status.html
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7305,6 +7305,9 @@
 
   case clang::TemplateArgument::Pack:
 return eTemplateArgumentKindPack;
+
+  case clang::TemplateArgument::StructuralValue:
+return eTemplateArgumentKindStructuralValue;
   }
   llvm_unreachable("Unhandled clang::TemplateArgument::ArgKind");
 }
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -849,6 +849,7 @@
   eTemplateArgumentKindExpression,
   eTemplateArgumentKindPack,
   eTemplateArgumentKindNullPtr,
+  eTemplateArgumentKindStructuralValue,
 };
 
 /// Type of match to be performed when looking for a formatter for a data type.
Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1053,13 +1053,21 @@
 
 
 
-  Class types as non-type template parameters
+  Class types as non-type template parameters
   https://wg21.link/p0732r2;>P0732R2
-  Partial
+  Clang 12
+
+ 
+  Generalized non-type template parameters of scalar type
+  https://wg21.link/p1907r1;>P1907R1
+  
+
+  Clang 17 (Partial)
+  Reference type template arguments referring to instantiation-dependent objects and subobjects
+  (i.e. declared inside a template but neither type- nor value-dependent) aren't fully supported.
+
+  
 
-   
-https://wg21.link/p1907r1;>P1907R1
-  
 
   Destroying operator delete
   https://wg21.link/p0722r3;>P0722R3
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -1463,6 +1463,9 @@
 return CXTemplateArgumentKind_NullPtr;
   case TemplateArgument::Integral:
 return CXTemplateArgumentKind_Integral;
+  case TemplateArgument::StructuralValue:
+// FIXME: Expose these values.
+return CXTemplateArgumentKind_Invalid;
   case TemplateArgument::Template:
 return CXTemplateArgumentKind_Template;
   case TemplateArgument::TemplateExpansion:
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1570,6 +1570,11 @@
   return Visit(MakeCXCursor(E, StmtParent, TU, RegionOfInterest));
 return false;
 
+  case TemplateArgument::StructuralValue:

[PATCH] D150136: [Clang] Change default triple to LLVM_HOST_TRIPLE for the CUDA toolchain

2023-05-08 Thread Joseph Huber 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 rGc2c917f7f668: [Clang] Change default triple to 
LLVM_HOST_TRIPLE for the CUDA toolchain (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150136

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -711,8 +711,7 @@
 /// system's default triple if not provided.
 NVPTXToolChain::NVPTXToolChain(const Driver , const llvm::Triple ,
const ArgList )
-: NVPTXToolChain(D, Triple,
- llvm::Triple(llvm::sys::getDefaultTargetTriple()), Args,
+: NVPTXToolChain(D, Triple, llvm::Triple(LLVM_HOST_TRIPLE), Args,
  /*Freestanding=*/true) {}
 
 llvm::opt::DerivedArgList *


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -711,8 +711,7 @@
 /// system's default triple if not provided.
 NVPTXToolChain::NVPTXToolChain(const Driver , const llvm::Triple ,
const ArgList )
-: NVPTXToolChain(D, Triple,
- llvm::Triple(llvm::sys::getDefaultTargetTriple()), Args,
+: NVPTXToolChain(D, Triple, llvm::Triple(LLVM_HOST_TRIPLE), Args,
  /*Freestanding=*/true) {}
 
 llvm::opt::DerivedArgList *
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] c2c917f - [Clang] Change default triple to LLVM_HOST_TRIPLE for the CUDA toolchain

2023-05-08 Thread Joseph Huber via cfe-commits

Author: Joseph Huber
Date: 2023-05-08T15:54:50-05:00
New Revision: c2c917f7f6680ec7a1214af9f5105c2beb9ba162

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

LOG: [Clang] Change default triple to LLVM_HOST_TRIPLE for the CUDA toolchain

When cross-compiling NVPTX we use the triple to indicate which paths to
search for the CUDA toolchain. Currently this uses the default target
triple. This might not be exactly correct, as this is the default triple
used to compile binaries, not the host system. We want the host triple
because it indicates which folders should hold CUDA.

Reviewed By: tra

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Cuda.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Cuda.cpp 
b/clang/lib/Driver/ToolChains/Cuda.cpp
index c0fcdad861994..37b0b9c2ed05b 100644
--- a/clang/lib/Driver/ToolChains/Cuda.cpp
+++ b/clang/lib/Driver/ToolChains/Cuda.cpp
@@ -711,8 +711,7 @@ NVPTXToolChain::NVPTXToolChain(const Driver , const 
llvm::Triple ,
 /// system's default triple if not provided.
 NVPTXToolChain::NVPTXToolChain(const Driver , const llvm::Triple ,
const ArgList )
-: NVPTXToolChain(D, Triple,
- llvm::Triple(llvm::sys::getDefaultTargetTriple()), Args,
+: NVPTXToolChain(D, Triple, llvm::Triple(LLVM_HOST_TRIPLE), Args,
  /*Freestanding=*/true) {}
 
 llvm::opt::DerivedArgList *



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


[PATCH] D150140: [NFC][CLANG] Fix Static Code Analysis Concerns

2023-05-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/utils/TableGen/SveEmitter.cpp:302
   unsigned Shift = llvm::countr_zero(Mask);
+  assert(Shift >= 64 && "Shift is out of encodable range");
   return (V << Shift) & Mask;

Shouldn't this be: `assert(Shift < 64 &&"...")`?

`expr.shift` (https://eel.is/c++draft/expr.shift) says:
```
The operands shall be of integral or unscoped enumeration type and integral 
promotions are performed.
The type of the result is that of the promoted left operand.
The behavior is undefined if the right operand is negative, or greater than or 
equal to the width of the promoted left operand.```

uint64 stays as an `unsigned long`, so it is still 64 bits, so the only invalid 
value for `Shift` is 64 (though >64 is 'nonsense', but only impossible because 
of `llvm::countr_zero`).

One thing to consider: I wonder if we should instead be changing the 'shift' to 
be:

`(V << (Shift % 64)) && Mask` ?  It looks like `arm_sve.td` has the `NoFlags` 
value as zero, which I think will end up going through here possibly (or at 
least, inserted into `FlagTypes`.

So I suspect an assert might not be sufficient, since a 64 bit shift is 
possible in that case (since a zero 'Mask' is the only case where `countr_zero` 
will end up being 64).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150140

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


[clang] ca06638 - [SYCL][NFC] Remove dead code

2023-05-08 Thread Elizabeth Andrews via cfe-commits

Author: Elizabeth Andrews
Date: 2023-05-08T13:08:23-07:00
New Revision: ca06638bbbf42a511d1be141fc7c547c7995ed29

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

LOG: [SYCL][NFC] Remove dead code

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaSYCL.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e28ec936e4c2..f65f8e3f2b50 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13957,20 +13957,6 @@ class Sema final {
   SemaDiagnosticBuilder SYCLDiagIfDeviceCode(SourceLocation Loc,
  unsigned DiagID);
 
-  /// Check whether we're allowed to call Callee from the current context.
-  ///
-  /// - If the call is never allowed in a semantically-correct program
-  ///   emits an error and returns false.
-  ///
-  /// - If the call is allowed in semantically-correct programs, but only if
-  ///   it's never codegen'ed, creates a deferred diagnostic to be emitted if
-  ///   and when the caller is codegen'ed, and returns true.
-  ///
-  /// - Otherwise, returns true without emitting any diagnostics.
-  ///
-  /// Adds Callee to DeviceCallGraph if we don't know if its caller will be
-  /// codegen'ed yet.
-  bool checkSYCLDeviceFunction(SourceLocation Loc, FunctionDecl *Callee);
   void deepTypeCheckForSYCLDevice(SourceLocation UsedAt,
   llvm::DenseSet Visited,
   ValueDecl *DeclToCheck);

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 31936bce7862..4efa1b408607 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -15672,9 +15672,6 @@ Sema::BuildCXXConstructExpr(SourceLocation 
ConstructLoc, QualType DeclInitType,
   MarkFunctionReferenced(ConstructLoc, Constructor);
   if (getLangOpts().CUDA && !CheckCUDACall(ConstructLoc, Constructor))
 return ExprError();
-  if (getLangOpts().SYCLIsDevice &&
-  !checkSYCLDeviceFunction(ConstructLoc, Constructor))
-return ExprError();
 
   return CheckForImmediateInvocation(
   CXXConstructExpr::Create(

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ec0f31e489d5..2e463346e70f 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -309,8 +309,6 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, 
ArrayRef Locs,
 if (getLangOpts().CUDA && !CheckCUDACall(Loc, FD))
   return true;
 
-if (getLangOpts().SYCLIsDevice && !checkSYCLDeviceFunction(Loc, FD))
-  return true;
   }
 
   if (auto *MD = dyn_cast(D)) {
@@ -18468,9 +18466,6 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, 
FunctionDecl *Func,
   if (getLangOpts().CUDA)
 CheckCUDACall(Loc, Func);
 
-  if (getLangOpts().SYCLIsDevice)
-checkSYCLDeviceFunction(Loc, Func);
-
   // If we need a definition, try to create one.
   if (NeedDefinition && !Func->getBody()) {
 runWithSufficientStackSpace(Loc, [&] {

diff  --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp
index f8c713c8545d..ca0254d29e7f 100644
--- a/clang/lib/Sema/SemaSYCL.cpp
+++ b/clang/lib/Sema/SemaSYCL.cpp
@@ -33,22 +33,6 @@ Sema::SemaDiagnosticBuilder 
Sema::SYCLDiagIfDeviceCode(SourceLocation Loc,
   return SemaDiagnosticBuilder(DiagKind, Loc, DiagID, FD, *this);
 }
 
-bool Sema::checkSYCLDeviceFunction(SourceLocation Loc, FunctionDecl *Callee) {
-  assert(getLangOpts().SYCLIsDevice &&
- "Should only be called during SYCL compilation");
-  assert(Callee && "Callee may not be null.");
-
-  // Errors in an unevaluated context don't need to be generated,
-  // so we can safely skip them.
-  if (isUnevaluatedContext() || isConstantEvaluated())
-return true;
-
-  SemaDiagnosticBuilder::Kind DiagKind = SemaDiagnosticBuilder::K_Nop;
-
-  return DiagKind != SemaDiagnosticBuilder::K_Immediate &&
- DiagKind != SemaDiagnosticBuilder::K_ImmediateWithCallStack;
-}
-
 static bool isZeroSizedArray(Sema , QualType Ty) {
   if (const auto *CAT = SemaRef.getASTContext().getAsConstantArrayType(Ty))
 return CAT->getSize() == 0;



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


[PATCH] D150140: [NFC][CLANG] Fix Static Code Analysis Concerns

2023-05-08 Thread Soumi Manna via Phabricator via cfe-commits
Manna created this revision.
Manna added reviewers: erichkeane, tahonermann.
Herald added a subscriber: ctetreau.
Herald added a project: All.
Manna requested review of this revision.
Herald added a project: clang.

Reported by Static Analyzer Tool, Coverity:

  Bad bit shift operation
  The operation may have an undefined behavior or yield an unexpected result.
  
  In ::​SVEEmitter::​encodeFlag(unsigned long long, llvm::​StringRef): 
A bit shift operation has a shift amount which is too large or has a negative 
value.

  // Returns the SVETypeFlags for a given value and mask.
uint64_t encodeFlag(uint64_t V, StringRef MaskName) const {
  auto It = FlagTypes.find(MaskName);
//Condition It != llvm::StringMap::const_iterator const(this->FlagTypes.end()), taking 
true branch.
  if (It != FlagTypes.end()) {
uint64_t Mask = It->getValue();
//return_constant: Function call llvm::countr_zero(Mask) may return 64.
//assignment: Assigning: Shift = llvm::countr_zero(Mask). The value of 
Shift is now 64.
unsigned Shift = llvm::countr_zero(Mask);

   //Bad bit shift operation (BAD_SHIFT)
   //large_shift: In expression V << Shift, left shifting by more than 63 
bits has undefined behavior. The shift amount, Shift, is 64.
return (V << Shift) & Mask;
  }
  llvm_unreachable("Unsupported flag");
}

This patch adds an assert to resolve the bug.

  


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150140

Files:
  clang/utils/TableGen/SveEmitter.cpp


Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -299,6 +299,7 @@
 if (It != FlagTypes.end()) {
   uint64_t Mask = It->getValue();
   unsigned Shift = llvm::countr_zero(Mask);
+  assert(Shift >= 64 && "Shift is out of encodable range");
   return (V << Shift) & Mask;
 }
 llvm_unreachable("Unsupported flag");


Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -299,6 +299,7 @@
 if (It != FlagTypes.end()) {
   uint64_t Mask = It->getValue();
   unsigned Shift = llvm::countr_zero(Mask);
+  assert(Shift >= 64 && "Shift is out of encodable range");
   return (V << Shift) & Mask;
 }
 llvm_unreachable("Unsupported flag");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D150137: [clang][dataflow][NFC] Remove `SkipPast` param from `getValue(const ValueDecl &)`.

2023-05-08 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision.
ymandel added a comment.
This revision is now accepted and ready to land.

Thanks! It's really exciting to see this concept being erased from the API!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150137

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


[PATCH] D150136: [Clang] Change default triple to LLVM_HOST_TRIPLE for the CUDA toolchain

2023-05-08 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

> right now all it's used for is HostTriple.isOSWindows()

OK.

In that case we may want to rename the parameter to `BuildHostTriple` to make 
it clear which host we have in mind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150136

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


[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

2023-05-08 Thread Jan Sjödin via Phabricator via cfe-commits
jsjodin added inline comments.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4376
+function_ref CustomMapperCB) {
+  auto EmitNonContiguousDescriptor = [&]() {
+MapInfosTy::StructNonContiguousInfo  =

I don't think we need a lambda function, it can be a regular method or static 
function?



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4576
+  if (Info.requiresDevicePointerInfo())
+DeviceAddrCB(I, BP, BPVal);
+

Do we need to check if DeviceAddrCB is null?



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4601
+  Value *MFunc = ConstantPointerNull::get(Builder.getInt8PtrTy());
+  if (Value *CustomMFunc = CustomMapperCB(I))
+MFunc = Builder.CreatePointerCast(CustomMFunc, Builder.getInt8PtrTy());

Check if CustomMapperCB is nullptr?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149872

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


[PATCH] D137872: Implement lambdas with inalloca parameters by forwarding to function without inalloca calling convention.

2023-05-08 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

I'm having a bit of trouble following how exactly the thunk creation is working 
here... do we generate different code depending on whether the call operator 
and/or the static invoker are referenced?  Why is the function in 
EmitLambdaInAllocaCallOpFn not getting defined using the normal CodeGenModule 
machinery?




Comment at: clang/lib/CodeGen/CGCall.cpp:767
+(opts & FnInfoOpts::IsDelegateCall) == FnInfoOpts::IsDelegateCall;
+  CGFunctionInfo::Profile(ID, isInstanceMethod, isChainCall, info, paramInfos,
   required, resultType, argTypes);

I think you need to pass isDelegateCall to CGFunctionInfo::Profile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137872

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


[PATCH] D150139: [clang-repl] Enable basic multiline support.

2023-05-08 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev created this revision.
v.g.vassilev added reviewers: junaire, sunho, aaron.ballman.
Herald added a project: All.
v.g.vassilev requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: jplehr, sstefan1.

This patch allows the users to use backslash to tell clang-repl that more input 
is coming. This would help support OpenMP directives which generally require to 
be in separate lines.


Repository:
  rC Clang

https://reviews.llvm.org/D150139

Files:
  clang/test/Interpreter/multiline.cpp
  clang/tools/clang-repl/ClangRepl.cpp


Index: clang/tools/clang-repl/ClangRepl.cpp
===
--- clang/tools/clang-repl/ClangRepl.cpp
+++ clang/tools/clang-repl/ClangRepl.cpp
@@ -113,28 +113,38 @@
   if (OptInputs.empty()) {
 llvm::LineEditor LE("clang-repl");
 // FIXME: Add LE.setListCompleter
+std::string Input;
 while (std::optional Line = LE.readLine()) {
-  if (*Line == R"(%quit)")
+  llvm::StringRef L = *Line;
+  L = L.trim();
+  if (L.endswith("\\")) {
+// FIXME: Support #ifdef X \ ...
+Input += L.drop_back(1);
+LE.setPrompt("clang-repl...   ");
+continue;
+  }
+
+  Input += L;
+
+  if (Input == R"(%quit)") {
 break;
-  if (*Line == R"(%undo)") {
+  } else if (Input == R"(%undo)") {
 if (auto Err = Interp->Undo()) {
   llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
   HasError = true;
 }
-continue;
-  }
-  if (Line->rfind("%lib ", 0) == 0) {
-if (auto Err = Interp->LoadDynamicLibrary(Line->data() + 5)) {
+  } else if (Input.rfind("%lib ", 0) == 0) {
+if (auto Err = Interp->LoadDynamicLibrary(Input.data() + 5)) {
   llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
   HasError = true;
 }
-continue;
-  }
-
-  if (auto Err = Interp->ParseAndExecute(*Line)) {
+  } else if (auto Err = Interp->ParseAndExecute(Input)) {
 llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
 HasError = true;
   }
+
+  Input = "";
+  LE.setPrompt("clang-repl> ");
 }
   }
 
Index: clang/test/Interpreter/multiline.cpp
===
--- /dev/null
+++ clang/test/Interpreter/multiline.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl | FileCheck %s
+
+extern "C" int printf(const char*,...);
+int i = \
+  12;
+
+printf("i=%d\n", i);
+// CHECK: i=12
+
+void f(int x) \ 
+{   \
+  printf("x=\
+  %d", x); \
+}
+f(i);
+// CHECK: x=12
+
+// FIXME: Support preprocessor directives.
+// #if 0 \
+//   #error "Can't be!" \
+// #endif
+


Index: clang/tools/clang-repl/ClangRepl.cpp
===
--- clang/tools/clang-repl/ClangRepl.cpp
+++ clang/tools/clang-repl/ClangRepl.cpp
@@ -113,28 +113,38 @@
   if (OptInputs.empty()) {
 llvm::LineEditor LE("clang-repl");
 // FIXME: Add LE.setListCompleter
+std::string Input;
 while (std::optional Line = LE.readLine()) {
-  if (*Line == R"(%quit)")
+  llvm::StringRef L = *Line;
+  L = L.trim();
+  if (L.endswith("\\")) {
+// FIXME: Support #ifdef X \ ...
+Input += L.drop_back(1);
+LE.setPrompt("clang-repl...   ");
+continue;
+  }
+
+  Input += L;
+
+  if (Input == R"(%quit)") {
 break;
-  if (*Line == R"(%undo)") {
+  } else if (Input == R"(%undo)") {
 if (auto Err = Interp->Undo()) {
   llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
   HasError = true;
 }
-continue;
-  }
-  if (Line->rfind("%lib ", 0) == 0) {
-if (auto Err = Interp->LoadDynamicLibrary(Line->data() + 5)) {
+  } else if (Input.rfind("%lib ", 0) == 0) {
+if (auto Err = Interp->LoadDynamicLibrary(Input.data() + 5)) {
   llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
   HasError = true;
 }
-continue;
-  }
-
-  if (auto Err = Interp->ParseAndExecute(*Line)) {
+  } else if (auto Err = Interp->ParseAndExecute(Input)) {
 llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
 HasError = true;
   }
+
+  Input = "";
+  LE.setPrompt("clang-repl> ");
 }
   }
 
Index: clang/test/Interpreter/multiline.cpp
===
--- /dev/null
+++ clang/test/Interpreter/multiline.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl | FileCheck %s
+
+extern "C" int printf(const char*,...);
+int i = \
+  12;
+
+printf("i=%d\n", i);
+// CHECK: i=12
+
+void f(int x) 

[PATCH] D150136: [Clang] Change default triple to LLVM_HOST_TRIPLE for the CUDA toolchain

2023-05-08 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D150136#4327570 , @tra wrote:

> The change may be an improvement, but we may still have a potential issue 
> here.
>
> E.g. ideally we may want to be able to cross-compile a CUDA app on a powerpc 
> or ARM build host targeting NVIDIA GPU on a x86 host. So, the compilation 
> tools would need to be found for the powerpc/arm host, but the the pair of 
> triples used during compilation would have to be x86 and nvptx.

So, this triple is only used for locating the CUDA library itself. In that case 
it's generally assumed that it will match whatever file structure the host 
computer is using. Specifically, right now all it's used for is 
`HostTriple.isOSWindows()`.

> In this situation the LLVM_HOST_TRIPLE would not be the right triple at all. 
> Does OpenMP currently handle the cross-compilation scenario above?

I don't think anyone's tried OpenMP with cross compilation. Most likely because 
it's only supported on Linux currently. I actually don't know what would happen 
if you tried.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150136

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


[PATCH] D150136: [Clang] Change default triple to LLVM_HOST_TRIPLE for the CUDA toolchain

2023-05-08 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

The change may be an improvement, but we may still have a potential issue here.

E.g. ideally we may want to be able to cross-compile a CUDA app on a powerpc or 
ARM build host targeting NVIDIA GPU on a x86 host. So, the compilation tools 
would need to be found for the powerpc/arm host, but the the pair of triples 
used during compilation would have to be x86 and nvptx.

In this situation the LLVM_HOST_TRIPLE would not be the right triple at all. 
Does OpenMP currently handle the cross-compilation scenario above?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150136

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


[PATCH] D147319: [clang-repl] Consider the scope spec in template lookups for deduction guides

2023-05-08 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added a comment.

Thanks! That was blocking our jupyter integration so I decided to go ahead here 
since we already missed the clang17 train...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147319

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


[PATCH] D147319: [clang-repl] Consider the scope spec in template lookups for deduction guides

2023-05-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

LGTM :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147319

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


[PATCH] D147920: [clang] Add test for CWG399

2023-05-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/CXX/drs/dr3xx.cpp:1439
+
+namespace dr399 { // dr399: 11
+  // NB: reuse dr244 test 

aaron.ballman wrote:
> shafik wrote:
> > Endill wrote:
> > > shafik wrote:
> > > > Endill wrote:
> > > > > Despite a couple of FIXME in CWG244 test (out of dozens of examples), 
> > > > > it claims full availability since Clang 11. I'd take a more 
> > > > > conservative approach, declaring partial support, but I think that 
> > > > > declaring different availability for the same test would bring 
> > > > > unnecessary confusion. So I followed CWG244 availability.
> > > > > 
> > > > > Alternative is to demote CWG244 to partial, but I'm not sure we 
> > > > > should go back on our claims for CWG support that has been out for so 
> > > > > long.
> > > > I think the bugs are not awful, we should file bug reports if we don't 
> > > > already have them. Some of them seem like they should be not too bad to 
> > > > fix.
> > > > 
> > > > CC @aaron.ballman to get a second opinion
> > > If we are to file bug reports, I'm not sure what wording makes those 
> > > examples ill-formed. Is it [[ 
> > > http://eel.is/c++draft/basic.lookup#qual.general-4.6 | qual.general-4.6 
> > > ]]: `The type-name that is or contains Q shall refer to its (original) 
> > > lookup context (ignoring cv-qualification) under the interpretation 
> > > established by at least one (successful) lookup performed.`? I interpret 
> > > it as requiring names to the left and to the right of `~` to be found in 
> > > the same scope (lookup context; `namespace dr244` in our case). Could it 
> > > actually mean that they have to refer to the same type?
> > I am not sure maybe @rsmith might be able to help us here.
> I think we want to start being more conservative with claiming support for 
> features and DRs, and that means being more honest with "partial" markings 
> (with comments as to WHY the support is only partial, what's still left to be 
> done, etc). I don't think it's a problem to say "we've discovered enough 
> issues with this that we no longer claim to support it" when that's accurate.
> 
> I don't think we have a hard and fast rule for when a bug is sufficiently 
> worrying to merit partial vs full support; it's going to depend on the 
> situation, I think. Failing to diagnose incorrect code is a different kind of 
> problem from diagnosing correct code from crashing bugs from etc. and it's 
> going to be up to the patch author and reviewers to make a value judgement. 
> But that's why I think it's fine for us to update the status when we learn 
> more information, too.
> 
> That said, when we do have partial support, we definitely need to file issues 
> to address the remaining bits at some point.
> 
> > I interpret it as requiring names to the left and to the right of ~ to be 
> > found in the same scope (lookup context; namespace dr244 in our case). 
> > Could it actually mean that they have to refer to the same type?
> 
> I've read that wording a few times now and can't make heads or tails of what 
> it's trying to say. Perhaps @rsmith or @hubert.reinterpretcast can help 
> illuminate us?
@Endill reminded me off-list that the FIXME comments here are existing 
comments; some of these test cases are lifted from the dr244 test cases. Given 
that and it's been a few weeks and we've not determine what issues to file, I 
think we should unblock this review as it makes forward progress on our test 
coverage for dr399. Filing issues would be good, but not a prerequisite for 
landing this. WDYT @shafik?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147920

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


[PATCH] D150137: [clang][dataflow][NFC] Remove `SkipPast` param from `getValue(const ValueDecl &)`.

2023-05-08 Thread Martin Böhme via Phabricator via cfe-commits
mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This parameter was already a no-op, so removing it doesn't change behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150137

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
  clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
  clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -489,8 +489,7 @@
 ASSERT_THAT(FooDecl, NotNull());
 
 auto GetFooValue = [FooDecl](const Environment ) {
-  return cast(
-  Env.getValue(*FooDecl, SkipPast::None)->getProperty("is_set"));
+  return cast(Env.getValue(*FooDecl)->getProperty("is_set"));
 };
 
 EXPECT_FALSE(Env1.flowConditionImplies(*GetFooValue(Env1)));
@@ -640,7 +639,7 @@
 ASSERT_THAT(FooDecl, NotNull());
 
 auto GetFooValue = [FooDecl](const Environment ) {
-  return Env.getValue(*FooDecl, SkipPast::None);
+  return Env.getValue(*FooDecl);
 };
 
 EXPECT_EQ(GetFooValue(Env1)->getProperty("has_value"),
@@ -685,7 +684,7 @@
 ASSERT_THAT(FooDecl, NotNull());
 
 auto GetFooValue = [FooDecl](const Environment ) {
-  return Env.getValue(*FooDecl, SkipPast::None);
+  return Env.getValue(*FooDecl);
 };
 
 EXPECT_EQ(GetFooValue(Env1)->getProperty("has_value"),
@@ -725,8 +724,7 @@
 
 const auto *FooLoc =
 cast(Env.getStorageLocation(*FooDecl));
-const auto *BarVal =
-cast(Env.getValue(*BarDecl, SkipPast::None));
+const auto *BarVal = cast(Env.getValue(*BarDecl));
 EXPECT_EQ(>getPointeeLoc(), FooLoc);
   });
 }
@@ -755,7 +753,7 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-const auto *FooVal = Env.getValue(*FooDecl, SkipPast::None);
+const auto *FooVal = Env.getValue(*FooDecl);
 EXPECT_EQ(FooVal->getProperty("has_value"),
   (true));
   });
@@ -804,13 +802,11 @@
 ASSERT_THAT(Results.keys(), UnorderedElementsAre("p1", "p2"));
 
 const Environment  = getEnvironmentAtAnnotation(Results, "p1");
-auto *FooVal1 =
-cast(Env1.getValue(*FooDecl, SkipPast::None));
+auto *FooVal1 = cast(Env1.getValue(*FooDecl));
 EXPECT_TRUE(Env1.flowConditionImplies(*FooVal1));
 
 const Environment  = getEnvironmentAtAnnotation(Results, "p2");
-auto *FooVal2 =
-cast(Env2.getValue(*FooDecl, SkipPast::None));
+auto *FooVal2 = cast(Env2.getValue(*FooDecl));
 EXPECT_FALSE(Env2.flowConditionImplies(*FooVal2));
   });
 }
@@ -837,13 +833,11 @@
 ASSERT_THAT(Results.keys(), UnorderedElementsAre("p1", "p2"));
 
 const Environment  = getEnvironmentAtAnnotation(Results, "p1");
-auto *FooVal1 =
-cast(Env1.getValue(*FooDecl, SkipPast::None));
+auto *FooVal1 = cast(Env1.getValue(*FooDecl));
 EXPECT_FALSE(Env1.flowConditionImplies(*FooVal1));
 
 const Environment  = getEnvironmentAtAnnotation(Results, "p2");
-auto *FooVal2 =
-cast(Env2.getValue(*FooDecl, SkipPast::None));
+auto *FooVal2 = cast(Env2.getValue(*FooDecl));
 EXPECT_TRUE(Env2.flowConditionImplies(*FooVal2));
   });
 }
@@ -867,7 +861,7 @@
 ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
 const Environment  = getEnvironmentAtAnnotation(Results, "p");
 
-auto *FooVal = cast(Env.getValue(*FooDecl, SkipPast::None));
+auto *FooVal = cast(Env.getValue(*FooDecl));
 EXPECT_TRUE(Env.flowConditionImplies(*FooVal));
   });
 }
@@ -896,14 +890,14 @@
 ASSERT_THAT(Results.keys(), UnorderedElementsAre("p1", "p2"));
 
 const Environment  = getEnvironmentAtAnnotation(Results, "p1");
-auto *FooVal1 = cast(Env1.getValue(*FooDecl, SkipPast::None));
-auto *BarVal1 = cast(Env1.getValue(*BarDecl, SkipPast::None));
+auto *FooVal1 = cast(Env1.getValue(*FooDecl));
+auto *BarVal1 = cast(Env1.getValue(*BarDecl));
 

[PATCH] D150072: [clang] Fix __is_trivially_equality_comparable for non-trivially-copyable types

2023-05-08 Thread Nikolas Klauser 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 rG465d48748131: [clang] Fix __is_trivially_equality_comparable 
for non-trivially-copyable types (authored by philnik).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150072

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3134,6 +3134,14 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), 
"");
 
+struct TriviallyEqualityComparableNonTriviallyCopyable {
+  TriviallyEqualityComparableNonTriviallyCopyable(const 
TriviallyEqualityComparableNonTriviallyCopyable&);
+  ~TriviallyEqualityComparableNonTriviallyCopyable();
+  bool operator==(const TriviallyEqualityComparableNonTriviallyCopyable&) 
const = default;
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableNonTriviallyCopyable));
+
 struct NotTriviallyEqualityComparableHasPadding {
   short i;
   int j;
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2663,7 +2663,8 @@
   return false;
   }
 
-  return Context.hasUniqueObjectRepresentations(CanonicalType);
+  return Context.hasUniqueObjectRepresentations(
+  CanonicalType, /*CheckIfTriviallyCopyable=*/false);
 }
 
 bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext ) const {


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3134,6 +3134,14 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), "");
 
+struct TriviallyEqualityComparableNonTriviallyCopyable {
+  TriviallyEqualityComparableNonTriviallyCopyable(const TriviallyEqualityComparableNonTriviallyCopyable&);
+  ~TriviallyEqualityComparableNonTriviallyCopyable();
+  bool operator==(const TriviallyEqualityComparableNonTriviallyCopyable&) const = default;
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableNonTriviallyCopyable));
+
 struct NotTriviallyEqualityComparableHasPadding {
   short i;
   int j;
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2663,7 +2663,8 @@
   return false;
   }
 
-  return Context.hasUniqueObjectRepresentations(CanonicalType);
+  return Context.hasUniqueObjectRepresentations(
+  CanonicalType, /*CheckIfTriviallyCopyable=*/false);
 }
 
 bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext ) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 465d487 - [clang] Fix __is_trivially_equality_comparable for non-trivially-copyable types

2023-05-08 Thread Nikolas Klauser via cfe-commits

Author: Nikolas Klauser
Date: 2023-05-08T12:07:28-07:00
New Revision: 465d487481313492e13435f3f03874b923b86ce3

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

LOG: [clang] Fix __is_trivially_equality_comparable for non-trivially-copyable 
types

Reviewed By: aaron.ballman

Spies: cfe-commits

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

Added: 


Modified: 
clang/lib/AST/Type.cpp
clang/test/SemaCXX/type-traits.cpp

Removed: 




diff  --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index ed481a4c0696..db369d127735 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2663,7 +2663,8 @@ bool QualType::isTriviallyEqualityComparableType(
   return false;
   }
 
-  return Context.hasUniqueObjectRepresentations(CanonicalType);
+  return Context.hasUniqueObjectRepresentations(
+  CanonicalType, /*CheckIfTriviallyCopyable=*/false);
 }
 
 bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext ) const {

diff  --git a/clang/test/SemaCXX/type-traits.cpp 
b/clang/test/SemaCXX/type-traits.cpp
index f9003b704099..75f172d1c345 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -3134,6 +3134,14 @@ struct TriviallyEqualityComparable {
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), 
"");
 
+struct TriviallyEqualityComparableNonTriviallyCopyable {
+  TriviallyEqualityComparableNonTriviallyCopyable(const 
TriviallyEqualityComparableNonTriviallyCopyable&);
+  ~TriviallyEqualityComparableNonTriviallyCopyable();
+  bool operator==(const TriviallyEqualityComparableNonTriviallyCopyable&) 
const = default;
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableNonTriviallyCopyable));
+
 struct NotTriviallyEqualityComparableHasPadding {
   short i;
   int j;



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


[PATCH] D150136: [Clang] Change default triple to LLVM_HOST_TRIPLE for the CUDA toolchain

2023-05-08 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 created this revision.
jhuber6 added reviewers: tra, yaxunl.
Herald added a subscriber: mattd.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

When cross-compiling NVPTX we use the triple to indicate which paths to
search for the CUDA toolchain. Currently this uses the default target
triple. This might not be exactly correct, as this is the default triple
used to compile binaries, not the host system. We want the host triple
because it indicates which folders should hold CUDA.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150136

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -711,8 +711,7 @@
 /// system's default triple if not provided.
 NVPTXToolChain::NVPTXToolChain(const Driver , const llvm::Triple ,
const ArgList )
-: NVPTXToolChain(D, Triple,
- llvm::Triple(llvm::sys::getDefaultTargetTriple()), Args,
+: NVPTXToolChain(D, Triple, llvm::Triple(LLVM_HOST_TRIPLE), Args,
  /*Freestanding=*/true) {}
 
 llvm::opt::DerivedArgList *


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -711,8 +711,7 @@
 /// system's default triple if not provided.
 NVPTXToolChain::NVPTXToolChain(const Driver , const llvm::Triple ,
const ArgList )
-: NVPTXToolChain(D, Triple,
- llvm::Triple(llvm::sys::getDefaultTargetTriple()), Args,
+: NVPTXToolChain(D, Triple, llvm::Triple(LLVM_HOST_TRIPLE), Args,
  /*Freestanding=*/true) {}
 
 llvm::opt::DerivedArgList *
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

2023-05-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

Ping:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149193

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


[PATCH] D150075: Fix PR#62594 : static lambda call operator is not convertible to function pointer on win32

2023-05-08 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik accepted this revision.
shafik 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/D150075/new/

https://reviews.llvm.org/D150075

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


[PATCH] D146399: [AIX][Clang][K] Create `-K` Option for AIX.

2023-05-08 Thread Michael Francis via Phabricator via cfe-commits
francii updated this revision to Diff 520455.
francii added a comment.

Don't claim `-k` when checking target


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146399

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/aix-ld.c
  clang/test/Driver/unsupported-target-K.c


Index: clang/test/Driver/unsupported-target-K.c
===
--- /dev/null
+++ clang/test/Driver/unsupported-target-K.c
@@ -0,0 +1,8 @@
+// Check powerpc64-unknown-linux-gnu. -K not supported.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc64-unknown-linux-gnu \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck --check-prefixes=CHECK-K-SUPPORT %s
+// CHECK-K-SUPPORT: clang: error: unsupported option '-K' for target 
'powerpc64-unknown-linux-gnu'
Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -1096,3 +1096,28 @@
 // CHECK-RELOCATABLE-NOT: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-RELOCATABLE-NOT: "-l{{.*}}"
 // CHECK-RELOCATABLE-NOT: "-L{{.*}}"
+
+// Check powerpc-ibm-aix7.1.0.0. -K is a passthrough linker option.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck --check-prefixes=CHECK-K %s
+// CHECK-K: "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-K: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-K: "{{.*}}ld{{(.exe)?}}"
+// CHECK-K: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-K: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-K: "-K"
+
+// Check powerpc-ibm-aix7.1.0.0. -K unused when not linking.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:-c \
+// RUN:   | FileCheck --check-prefixes=CHECK-K-UNUSED %s
+// CHECK-K-UNUSED: clang: warning: -K: 'linker' input unused 
[-Wunused-command-line-argument]
+>>> 8f57d6ae6c1a ([AIX][Clang][K] Create `-K` Option for AIX.)
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6326,6 +6326,11 @@
 }
   }
 
+  if (Arg *A = Args.getLastArgNoClaim(options::OPT_K);
+  A && !TC.getTriple().isOSAIX())
+D.Diag(diag::err_drv_unsupported_opt_for_target)
+<< A->getAsString(Args) << TripleStr;
+
   if (Args.getLastArg(options::OPT_fapple_kext) ||
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
 CmdArgs.push_back("-fapple-kext");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3421,6 +3421,7 @@
   HelpText<"Overlay the virtual filesystem described by file over the real 
file system. "
"Additionally, pass this overlay file to the linker if it supports 
it">;
 def imultilib : Separate<["-"], "imultilib">, Group;
+def K : Flag<["-"], "K">, Flags<[LinkerInput]>;
 def keep__private__externs : Flag<["-"], "keep_private_externs">;
 def l : JoinedOrSeparate<["-"], "l">, Flags<[LinkerInput, RenderJoined]>,
 Group;


Index: clang/test/Driver/unsupported-target-K.c
===
--- /dev/null
+++ clang/test/Driver/unsupported-target-K.c
@@ -0,0 +1,8 @@
+// Check powerpc64-unknown-linux-gnu. -K not supported.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc64-unknown-linux-gnu \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck --check-prefixes=CHECK-K-SUPPORT %s
+// CHECK-K-SUPPORT: clang: error: unsupported option '-K' for target 'powerpc64-unknown-linux-gnu'
Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -1096,3 +1096,28 @@
 // CHECK-RELOCATABLE-NOT: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-RELOCATABLE-NOT: "-l{{.*}}"
 // CHECK-RELOCATABLE-NOT: "-L{{.*}}"
+
+// Check powerpc-ibm-aix7.1.0.0. -K is a passthrough linker option.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-K \
+// RUN:   | FileCheck --check-prefixes=CHECK-K %s
+// CHECK-K: "-cc1" 

[PATCH] D149612: [Sema] avoid merge error type

2023-05-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane requested changes to this revision.
erichkeane added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Sema/SemaType.cpp:2583
+if (ArraySize->containsErrors()) {
+  RecoveryExpr *RE = RecoveryExpr::Create(
+  Context, ArraySize->getType(), ArraySize->getBeginLoc(),

Actually thinking further... rather than create a NEW RecoveryExpr, is there a 
problem KEEPING the ArraySize expression?  It'd likely give more 
information/keep more AST consistency.

ALSO, creating this as size-0 has some implications we probably don't want.  I 
wonder if a different for the placeholder would be better?  

I'D ALSO suggest hoisting this ArraySize->containsError out of the else-if and 
into its own branch (rather than inside the dependent checks).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149612

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


[clang] 96bc786 - [Clang] Update warning on some designator initializer cases involving unions

2023-05-08 Thread Shafik Yaghmour via cfe-commits

Author: Shafik Yaghmour
Date: 2023-05-08T11:14:33-07:00
New Revision: 96bc78631f16fe5ce2e7e6000b74d790b32f7a16

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

LOG: [Clang] Update warning on some designator initializer cases involving 
unions

Currently when using designated initializers in C++ we have a few
extension. Two extension which are dangerous involved assigning to
multiple members of union which will likely be a mistake since unions
can only have one active member. I have updated to be a warning by
default.

The second case if when we assign to multiple union members and one of
the previous members had a non-trivial destructor, which could lead to
leaking resources. This one is now an error by default.

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

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaInit.cpp
clang/test/SemaCXX/cxx2b-designated-initializers.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c36828fc05da2..1d30d6f2102a4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -186,6 +186,8 @@ def warn_initializer_overrides : Warning<
   "this subobject">, InGroup;
 def ext_initializer_overrides : ExtWarn,
   InGroup, SFINAEFailure;
+def ext_initializer_union_overrides : 
ExtWarn,
+  InGroup, DefaultError, SFINAEFailure;
 def err_initializer_overrides_destructed : Error<
   "initializer would partially override prior initialization of object of "
   "type %1 with non-trivial destruction">;

diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index d27cd6b64c62a..3db70223bb8ba 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -394,12 +394,15 @@ class InitListChecker {
 
   /// Diagnose that OldInit (or part thereof) has been overridden by NewInit.
   void diagnoseInitOverride(Expr *OldInit, SourceRange NewInitRange,
+bool UnionOverride = false,
 bool FullyOverwritten = true) {
 // Overriding an initializer via a designator is valid with C99 designated
 // initializers, but ill-formed with C++20 designated initializers.
-unsigned DiagID = SemaRef.getLangOpts().CPlusPlus
-  ? diag::ext_initializer_overrides
-  : diag::warn_initializer_overrides;
+unsigned DiagID =
+SemaRef.getLangOpts().CPlusPlus
+? (UnionOverride ? diag::ext_initializer_union_overrides
+ : diag::ext_initializer_overrides)
+: diag::warn_initializer_overrides;
 
 if (InOverloadResolution && SemaRef.getLangOpts().CPlusPlus) {
   // In overload resolution, we have to strictly enforce the rules, and so
@@ -2546,6 +2549,7 @@ InitListChecker::CheckDesignatedInitializer(const 
InitializedEntity ,
 // subobject [0].b.
 diagnoseInitOverride(ExistingInit,
  SourceRange(D->getBeginLoc(), DIE->getEndLoc()),
+ /*UnionOverride=*/false,
  /*FullyOverwritten=*/false);
 
 if (!VerifyOnly) {
@@ -2691,7 +2695,10 @@ InitListChecker::CheckDesignatedInitializer(const 
InitializedEntity ,
   if (ExistingInit) {
 // We're about to throw away an initializer, emit warning.
 diagnoseInitOverride(
-ExistingInit, SourceRange(D->getBeginLoc(), DIE->getEndLoc()));
+ExistingInit, SourceRange(D->getBeginLoc(), DIE->getEndLoc()),
+/*UnionOverride=*/true,
+/*FullyOverwritten=*/SemaRef.getLangOpts().CPlusPlus ? false
+ : true);
   }
 
   // remove existing initializer

diff  --git a/clang/test/SemaCXX/cxx2b-designated-initializers.cpp 
b/clang/test/SemaCXX/cxx2b-designated-initializers.cpp
index 3774565513673..5588926f419a9 100644
--- a/clang/test/SemaCXX/cxx2b-designated-initializers.cpp
+++ b/clang/test/SemaCXX/cxx2b-designated-initializers.cpp
@@ -19,3 +19,27 @@ void g(void) {
 }
 
 } // end namespace PR61118
+
+namespace GH62156 {
+union U1 {
+   int x;
+   float y;
+};
+
+struct NonTrivial {
+  NonTrivial();
+  ~NonTrivial();
+};
+
+union U2 {
+   NonTrivial x;
+   float y;
+};
+
+void f() {
+   U1 u{.x=2,  // expected-note {{previous initialization is here}}
+.y=1}; // expected-error {{initializer partially overrides prior 
initialization of this subobject}}
+   new U2{.x = NonTrivial{}, // expected-note {{previous initialization is 

[PATCH] D149694: [Clang] Update warning on some designator initializer cases involving unions

2023-05-08 Thread Shafik Yaghmour via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG96bc78631f16: [Clang] Update warning on some designator 
initializer cases involving unions (authored by shafik).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149694

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx2b-designated-initializers.cpp


Index: clang/test/SemaCXX/cxx2b-designated-initializers.cpp
===
--- clang/test/SemaCXX/cxx2b-designated-initializers.cpp
+++ clang/test/SemaCXX/cxx2b-designated-initializers.cpp
@@ -19,3 +19,27 @@
 }
 
 } // end namespace PR61118
+
+namespace GH62156 {
+union U1 {
+   int x;
+   float y;
+};
+
+struct NonTrivial {
+  NonTrivial();
+  ~NonTrivial();
+};
+
+union U2 {
+   NonTrivial x;
+   float y;
+};
+
+void f() {
+   U1 u{.x=2,  // expected-note {{previous initialization is here}}
+.y=1}; // expected-error {{initializer partially overrides prior 
initialization of this subobject}}
+   new U2{.x = NonTrivial{}, // expected-note {{previous initialization is 
here}}
+  .y=1}; // expected-error {{initializer would partially override 
prior initialization of object of type 'NonTrivial' with non-trivial 
destruction}}
+}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -394,12 +394,15 @@
 
   /// Diagnose that OldInit (or part thereof) has been overridden by NewInit.
   void diagnoseInitOverride(Expr *OldInit, SourceRange NewInitRange,
+bool UnionOverride = false,
 bool FullyOverwritten = true) {
 // Overriding an initializer via a designator is valid with C99 designated
 // initializers, but ill-formed with C++20 designated initializers.
-unsigned DiagID = SemaRef.getLangOpts().CPlusPlus
-  ? diag::ext_initializer_overrides
-  : diag::warn_initializer_overrides;
+unsigned DiagID =
+SemaRef.getLangOpts().CPlusPlus
+? (UnionOverride ? diag::ext_initializer_union_overrides
+ : diag::ext_initializer_overrides)
+: diag::warn_initializer_overrides;
 
 if (InOverloadResolution && SemaRef.getLangOpts().CPlusPlus) {
   // In overload resolution, we have to strictly enforce the rules, and so
@@ -2546,6 +2549,7 @@
 // subobject [0].b.
 diagnoseInitOverride(ExistingInit,
  SourceRange(D->getBeginLoc(), DIE->getEndLoc()),
+ /*UnionOverride=*/false,
  /*FullyOverwritten=*/false);
 
 if (!VerifyOnly) {
@@ -2691,7 +2695,10 @@
   if (ExistingInit) {
 // We're about to throw away an initializer, emit warning.
 diagnoseInitOverride(
-ExistingInit, SourceRange(D->getBeginLoc(), DIE->getEndLoc()));
+ExistingInit, SourceRange(D->getBeginLoc(), DIE->getEndLoc()),
+/*UnionOverride=*/true,
+/*FullyOverwritten=*/SemaRef.getLangOpts().CPlusPlus ? false
+ : true);
   }
 
   // remove existing initializer
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -186,6 +186,8 @@
   "this subobject">, InGroup;
 def ext_initializer_overrides : ExtWarn,
   InGroup, SFINAEFailure;
+def ext_initializer_union_overrides : 
ExtWarn,
+  InGroup, DefaultError, SFINAEFailure;
 def err_initializer_overrides_destructed : Error<
   "initializer would partially override prior initialization of object of "
   "type %1 with non-trivial destruction">;


Index: clang/test/SemaCXX/cxx2b-designated-initializers.cpp
===
--- clang/test/SemaCXX/cxx2b-designated-initializers.cpp
+++ clang/test/SemaCXX/cxx2b-designated-initializers.cpp
@@ -19,3 +19,27 @@
 }
 
 } // end namespace PR61118
+
+namespace GH62156 {
+union U1 {
+   int x;
+   float y;
+};
+
+struct NonTrivial {
+  NonTrivial();
+  ~NonTrivial();
+};
+
+union U2 {
+   NonTrivial x;
+   float y;
+};
+
+void f() {
+   U1 u{.x=2,  // expected-note {{previous initialization is here}}
+.y=1}; // expected-error {{initializer partially overrides prior initialization of this subobject}}
+   new U2{.x = NonTrivial{}, // expected-note {{previous initialization is here}}
+  .y=1}; // expected-error {{initializer would partially override prior initialization of object of type 

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

2023-05-08 Thread Guillot Tony via Phabricator via cfe-commits
to268 updated this revision to Diff 520447.
to268 added a comment.

Fixed auto char arrays with explicit brackets

  c
  auto foo[] = "bar";


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/C/C2x/n3007.c
  clang/test/CodeGen/auto.c
  clang/test/Parser/c2x-auto.c
  clang/test/Sema/c2x-auto.c
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -1195,7 +1195,7 @@
 
   Type inference for object declarations
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3007.htm;>N3007
-  No
+  Clang 17
 
 
   constexpr for object definitions
Index: clang/test/Sema/c2x-auto.c
===
--- /dev/null
+++ clang/test/Sema/c2x-auto.c
@@ -0,0 +1,74 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c2x %s
+
+void test_basic_types(void) {
+  auto undefined; // expected-error {{declaration of variable 'undefined' with deduced type 'auto' requires an initializer}}
+  auto auto_int = 4;
+  auto auto_long = 4UL;
+}
+
+void test_sizeof_typeof(void) {
+  auto auto_size = sizeof(auto);  // expected-error {{expected expression}}
+  typeof(auto) tpof = 4;  // expected-error {{expected expression}}
+}
+
+void test_casts(void) {
+  auto int_cast = (int)(4 + 3);
+  auto double_cast = (double)(1 / 3);
+  auto long_cast = (long)(4UL + 3UL);
+  auto auto_cast = (auto)(4 + 3); // expected-error {{expected expression}}
+}
+
+void test_compound_literral(void) {
+  auto int_cl = (int){13};
+  auto double_cl = (double){2.5};
+  auto array[] = { 1, 2, 3 }; // expected-error {{cannot use 'auto' with initializer list in C}}
+
+  // FIXME: This should be accepted per C2x 6.5.2.5p4
+  auto auto_cl = (auto){13};  // expected-error {{expected expression}}
+}
+
+void test_array_pointers(void) {
+  double array[3] = { 0 };
+  auto a = array;
+  auto b = 
+}
+
+void test_qualifiers(const int y) {
+  const auto a = 12;
+  auto b = y;
+  static auto c = 1UL;
+  int* pa =  // expected-warning {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}}
+  const int* pb = 
+  int* pc =  // expected-warning {{incompatible pointer types initializing 'int *' with an expression of type 'unsigned long *'}}
+
+}
+
+void test_strings(void) {
+  auto str = "this is a string";
+  auto str2[] = "this is a string";
+  auto (str3) = "this is a string";
+  auto (((str4))) = "this is a string";
+}
+
+void test_pointers(void) {
+  auto a = 12;
+  auto *ptr = 
+  auto *str = "this is a string";
+  const auto *str2 = "this is a string";
+  auto *b = 
+  *b =  // expected-error {{incompatible pointer to integer conversion assigning to 'int' from 'int *'; remove &}}
+  auto nptr = nullptr;
+}
+
+void test_scopes(void) {
+  double a = 7;
+  double b = 9;
+  {
+auto a = a * a; // expected-error {{variable 'a' declared with deduced type 'auto' cannot appear in its own initializer}} \
+   expected-error {{variable 'a' declared with deduced type 'auto' cannot appear in its own initializer}}
+  }
+  {
+auto b = a * a;
+auto a = b;
+  }
+}
Index: clang/test/Parser/c2x-auto.c
===
--- /dev/null
+++ clang/test/Parser/c2x-auto.c
@@ -0,0 +1,123 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c2x -std=c2x %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,c17 -std=c17 %s
+
+#define AUTO_MACRO(_NAME, ARG, ARG2, ARG3) \
+auto _NAME = ARG + (ARG2 / ARG3);
+
+struct S {
+  int a;
+  auto b;   // c2x-error {{'auto' not allowed in struct member}} \
+   c17-error {{type name does not allow storage class to be specified}} \
+   c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
+  union {
+char c;
+auto smth;  // c2x-error {{'auto' not allowed in union member}} \
+   c17-error {{type name does not allow storage class to be specified}} \
+   c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
+  } u;
+};
+
+enum E : auto { // c2x-error {{'auto' not allowed here}} \
+   c17-error {{expected a type}} \
+   c17-error {{type name does not allow storage class to be specified}}
+  One,
+  Two,
+  Tree,
+};
+
+auto basic_usage(auto auto) {   // c2x-error {{'auto' not allowed in function prototype}} \
+   c2x-error {{'auto' not allowed in function return type}} \
+   c2x-error {{cannot combine with 

[PATCH] D144999: [RFC][MC][MachO]Only emits compact-unwind format for "canonical" personality symbols. For the rest, use DWARFs.

2023-05-08 Thread Vy Nguyen via Phabricator via cfe-commits
oontvoo added a comment.

Hi all! The CI builds are all clean now. Any further comment/review on this? 
Would love to close this out soon. Thanks! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144999

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


[PATCH] D147319: [clang-repl] Consider the scope spec in template lookups for deduction guides

2023-05-08 Thread Vassil Vassilev via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2c4620c1dadc: [clang-repl] Consider the scope spec in 
template lookups for deduction guides. (authored by v.g.vassilev).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147319

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Interpreter/disambiguate-decl-stmt.cpp


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -7,6 +7,10 @@
 
 // Decls which are hard to disambiguate
 
+// Templates
+namespace ns1 { template void tmplt(T &) {}}
+int arg_tmplt = 12; ns1::tmplt(arg_tmplt);
+
 // ParseStatementOrDeclaration returns multiple statements.
 #ifdef MS
 int g_bFlag = 1;
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -316,9 +316,8 @@
 }
 
 bool Sema::isDeductionGuideName(Scope *S, const IdentifierInfo ,
-SourceLocation NameLoc,
-ParsedTemplateTy *Template) {
-  CXXScopeSpec SS;
+SourceLocation NameLoc, CXXScopeSpec ,
+ParsedTemplateTy *Template /*=nullptr*/) {
   bool MemberOfUnknownSpecialization = false;
 
   // We could use redeclaration lookup here, but we don't need to: the
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -74,9 +74,8 @@
   switch (Tok.getKind()) {
   case tok::identifier: {
 IdentifierInfo *II = Tok.getIdentifierInfo();
-bool isDeductionGuide =
-Actions.isDeductionGuideName(getCurScope(), *II, Tok.getLocation(),
- /*Template=*/nullptr);
+bool isDeductionGuide = Actions.isDeductionGuideName(
+getCurScope(), *II, Tok.getLocation(), SS, /*Template=*/nullptr);
 if (Actions.isCurrentClassName(*II, getCurScope(), ) ||
 isDeductionGuide) {
   if (isConstructorDeclarator(/*Unqualified=*/SS.isEmpty(),
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -2911,9 +2911,9 @@
   if (!Ty)
 return true;
   Result.setConstructorName(Ty, IdLoc, IdLoc);
-} else if (getLangOpts().CPlusPlus17 &&
-   AllowDeductionGuide && SS.isEmpty() &&
-   Actions.isDeductionGuideName(getCurScope(), *Id, IdLoc,
+} else if (getLangOpts().CPlusPlus17 && AllowDeductionGuide &&
+   SS.isEmpty() &&
+   Actions.isDeductionGuideName(getCurScope(), *Id, IdLoc, SS,
 )) {
   // We have parsed a template-name naming a deduction guide.
   Result.setDeductionGuideName(TemplateName, IdLoc);
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -3696,11 +3696,12 @@
 
   // Likewise, if this is a context where the identifier could be a 
template
   // name, check whether this is a deduction guide declaration.
+  CXXScopeSpec SS;
   if (getLangOpts().CPlusPlus17 &&
   (DSContext == DeclSpecContext::DSC_class ||
DSContext == DeclSpecContext::DSC_top_level) &&
   Actions.isDeductionGuideName(getCurScope(), *Tok.getIdentifierInfo(),
-   Tok.getLocation()) &&
+   Tok.getLocation(), SS) &&
   isConstructorDeclarator(/*Unqualified*/ true,
   /*DeductionGuide*/ true))
 goto DoneWithDeclSpec;
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -8081,7 +8081,7 @@
   /// Determine whether a particular identifier might be the name in a C++1z
   /// deduction-guide declaration.
   bool isDeductionGuideName(Scope *S, const IdentifierInfo ,
-SourceLocation NameLoc,
+SourceLocation NameLoc, CXXScopeSpec ,
 ParsedTemplateTy *Template = nullptr);
 
   bool 

[clang] 2c4620c - [clang-repl] Consider the scope spec in template lookups for deduction guides.

2023-05-08 Thread Vassil Vassilev via cfe-commits

Author: Vassil Vassilev
Date: 2023-05-08T17:54:56Z
New Revision: 2c4620c1dadc032f968ce0aa835a441f268a8cdb

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

LOG: [clang-repl] Consider the scope spec in template lookups for deduction 
guides.

isDeductionGuideName looks up the underlying template and if the template name
is qualified we miss that qualification resulting in an error. This issue
resurfaced in clang-repl where we call isDeductionGuideName more often to
distinguish between if we had a statement or declaration.

This patch passes the CXXScopeSpec information down to LookupTemplateName to
make the lookup more precise.

Differential revision: https://reviews.llvm.org/D147319

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Parse/ParseTentative.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/Interpreter/disambiguate-decl-stmt.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c6facd4e9e4af..e28ec936e4c2f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8081,7 +8081,7 @@ class Sema final {
   /// Determine whether a particular identifier might be the name in a C++1z
   /// deduction-guide declaration.
   bool isDeductionGuideName(Scope *S, const IdentifierInfo ,
-SourceLocation NameLoc,
+SourceLocation NameLoc, CXXScopeSpec ,
 ParsedTemplateTy *Template = nullptr);
 
   bool DiagnoseUnknownTemplateName(const IdentifierInfo ,

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index e28d609464123..2bbe2ba82139d 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -3696,11 +3696,12 @@ void Parser::ParseDeclarationSpecifiers(
 
   // Likewise, if this is a context where the identifier could be a 
template
   // name, check whether this is a deduction guide declaration.
+  CXXScopeSpec SS;
   if (getLangOpts().CPlusPlus17 &&
   (DSContext == DeclSpecContext::DSC_class ||
DSContext == DeclSpecContext::DSC_top_level) &&
   Actions.isDeductionGuideName(getCurScope(), *Tok.getIdentifierInfo(),
-   Tok.getLocation()) &&
+   Tok.getLocation(), SS) &&
   isConstructorDeclarator(/*Unqualified*/ true,
   /*DeductionGuide*/ true))
 goto DoneWithDeclSpec;

diff  --git a/clang/lib/Parse/ParseExprCXX.cpp 
b/clang/lib/Parse/ParseExprCXX.cpp
index 8f56316cefcdf..123cf432b0969 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -2911,9 +2911,9 @@ bool Parser::ParseUnqualifiedId(CXXScopeSpec , 
ParsedType ObjectType,
   if (!Ty)
 return true;
   Result.setConstructorName(Ty, IdLoc, IdLoc);
-} else if (getLangOpts().CPlusPlus17 &&
-   AllowDeductionGuide && SS.isEmpty() &&
-   Actions.isDeductionGuideName(getCurScope(), *Id, IdLoc,
+} else if (getLangOpts().CPlusPlus17 && AllowDeductionGuide &&
+   SS.isEmpty() &&
+   Actions.isDeductionGuideName(getCurScope(), *Id, IdLoc, SS,
 )) {
   // We have parsed a template-name naming a deduction guide.
   Result.setDeductionGuideName(TemplateName, IdLoc);

diff  --git a/clang/lib/Parse/ParseTentative.cpp 
b/clang/lib/Parse/ParseTentative.cpp
index 934087e59b809..02aa59ec6fa1f 100644
--- a/clang/lib/Parse/ParseTentative.cpp
+++ b/clang/lib/Parse/ParseTentative.cpp
@@ -74,9 +74,8 @@ bool Parser::isCXXDeclarationStatement(
   switch (Tok.getKind()) {
   case tok::identifier: {
 IdentifierInfo *II = Tok.getIdentifierInfo();
-bool isDeductionGuide =
-Actions.isDeductionGuideName(getCurScope(), *II, Tok.getLocation(),
- /*Template=*/nullptr);
+bool isDeductionGuide = Actions.isDeductionGuideName(
+getCurScope(), *II, Tok.getLocation(), SS, /*Template=*/nullptr);
 if (Actions.isCurrentClassName(*II, getCurScope(), ) ||
 isDeductionGuide) {
   if (isConstructorDeclarator(/*Unqualified=*/SS.isEmpty(),

diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index ca1b86c06097e..4fe4b9192ecd3 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -316,9 +316,8 @@ TemplateNameKind Sema::isTemplateName(Scope *S,
 }
 
 bool Sema::isDeductionGuideName(Scope *S, const IdentifierInfo ,
-SourceLocation NameLoc,
-  

[PATCH] D147319: [clang-repl] Consider the scope spec in template lookups for deduction guides

2023-05-08 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added a comment.

Let's rely on a post-commit review here.


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

https://reviews.llvm.org/D147319

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


[PATCH] D149280: [clang-tidy] Add modernize-printf-to-std-print check

2023-05-08 Thread Mike Crowe via Phabricator via cfe-commits
mikecrowe updated this revision to Diff 520439.
mikecrowe added a comment.

Address many more review comments, including:

- Only add casts for signed/unsigned discrepancy if StrictMode option is set.
- Use IncludeInserter to add  or other required header.

Review comments still outstanding:

- Use println if format string ends in `\n`.
- Remove c_str() as part of the same check (I'm not really sure how to do this, 
are there any other checks that I should look at for inspiration?)
- Emit warnings to explain why if conversion is not possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149280

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.h
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
  clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cstdio
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/stdio.h
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-absl.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-fmt.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp
@@ -0,0 +1,1049 @@
+// RUN: %check_clang_tidy -check-suffixes=,STRICT \
+// RUN:   -std=c++2b %s modernize-use-std-print %t -- \
+// RUN:   -config="{CheckOptions: [{key: StrictMode, value: true}]}" \
+// RUN:   -- -isystem %clang_tidy_headers
+// RUN: %check_clang_tidy -check-suffixes=,NOTSTRICT \
+// RUN:   -std=c++2b %s modernize-use-std-print %t -- \
+// RUN:   -config="{CheckOptions: [{key: StrictMode, value: false}]}" \
+// RUN:   -- -isystem %clang_tidy_headers
+#include 
+// CHECK-FIXES: #include 
+#include 
+#include 
+
+void printf_simple() {
+  printf("Hello");
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::print("Hello");
+}
+
+void fprintf_simple() {
+  fprintf(stderr, "Hello");
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'fprintf' [modernize-use-std-print]
+  // CHECK-FIXES: std::print(stderr, "Hello");
+}
+
+void std_printf_simple() {
+  std::printf("std::Hello");
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::print("std::Hello");
+}
+
+void printf_escape() {
+  printf("before \t");
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::print("before \t");
+
+  printf("\n after");
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::print("\n after");
+
+  printf("before \a after");
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::print("before \a after");
+
+  printf("Bell\a%dBackspace\bFF%s\fNewline\nCR\rTab\tVT\vEscape\x1b\x07%d", 42, "string", 99);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::print("Bell\a{}Backspace\bFF{}\fNewline\nCR\rTab\tVT\vEscape\x1b\a{}", 42, "string", 99);
+
+  printf("not special \x1b\x01\x7f");
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::print("not special \x1b\x01\x7f");
+}
+
+void printf_percent() {
+  printf("before %%");
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::print("before %");
+
+  printf("%% after");
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::print("% after");
+
+  printf("before %% after");
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::print("before % after");
+
+  printf("Hello %% and another %%");
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print]
+  // CHECK-FIXES: std::print("Hello % and another 

[PATCH] D149981: [clang][AST][NFC] Factor out check for structural equivalence of names.

2023-05-08 Thread Aaron Ballman 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 rG94252713f9a7: [clang][AST][NFC] Factor out check for 
structural equivalence of names. (authored by davidstone, committed by 
aaron.ballman).

Changed prior to commit:
  https://reviews.llvm.org/D149981?vs=519945=520434#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149981

Files:
  clang/lib/AST/ASTStructuralEquivalence.cpp


Index: clang/lib/AST/ASTStructuralEquivalence.cpp
===
--- clang/lib/AST/ASTStructuralEquivalence.cpp
+++ clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1453,19 +1453,23 @@
   return true;
 }
 
+static bool NameIsStructurallyEquivalent(const TagDecl , const TagDecl ) 
{
+  auto GetName = [](const TagDecl ) -> const IdentifierInfo * {
+if (const IdentifierInfo *Name = D.getIdentifier())
+  return Name;
+if (const TypedefNameDecl *TypedefName = D.getTypedefNameForAnonDecl())
+  return TypedefName->getIdentifier();
+return nullptr;
+  };
+  return IsStructurallyEquivalent(GetName(D1), GetName(D2));
+}
+
 /// Determine structural equivalence of two records.
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
  RecordDecl *D1, RecordDecl *D2) {
-
-  // Check for equivalent structure names.
-  IdentifierInfo *Name1 = D1->getIdentifier();
-  if (!Name1 && D1->getTypedefNameForAnonDecl())
-Name1 = D1->getTypedefNameForAnonDecl()->getIdentifier();
-  IdentifierInfo *Name2 = D2->getIdentifier();
-  if (!Name2 && D2->getTypedefNameForAnonDecl())
-Name2 = D2->getTypedefNameForAnonDecl()->getIdentifier();
-  if (!IsStructurallyEquivalent(Name1, Name2))
+  if (!NameIsStructurallyEquivalent(*D1, *D2)) {
 return false;
+  }
 
   if (D1->isUnion() != D2->isUnion()) {
 if (Context.Complain) {
@@ -1727,16 +1731,9 @@
 /// Determine structural equivalence of two enums.
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
  EnumDecl *D1, EnumDecl *D2) {
-
-  // Check for equivalent enum names.
-  IdentifierInfo *Name1 = D1->getIdentifier();
-  if (!Name1 && D1->getTypedefNameForAnonDecl())
-Name1 = D1->getTypedefNameForAnonDecl()->getIdentifier();
-  IdentifierInfo *Name2 = D2->getIdentifier();
-  if (!Name2 && D2->getTypedefNameForAnonDecl())
-Name2 = D2->getTypedefNameForAnonDecl()->getIdentifier();
-  if (!IsStructurallyEquivalent(Name1, Name2))
+  if (!NameIsStructurallyEquivalent(*D1, *D2)) {
 return false;
+  }
 
   // Compare the definitions of these two enums. If either or both are
   // incomplete (i.e. forward declared), we assume that they are equivalent.


Index: clang/lib/AST/ASTStructuralEquivalence.cpp
===
--- clang/lib/AST/ASTStructuralEquivalence.cpp
+++ clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1453,19 +1453,23 @@
   return true;
 }
 
+static bool NameIsStructurallyEquivalent(const TagDecl , const TagDecl ) {
+  auto GetName = [](const TagDecl ) -> const IdentifierInfo * {
+if (const IdentifierInfo *Name = D.getIdentifier())
+  return Name;
+if (const TypedefNameDecl *TypedefName = D.getTypedefNameForAnonDecl())
+  return TypedefName->getIdentifier();
+return nullptr;
+  };
+  return IsStructurallyEquivalent(GetName(D1), GetName(D2));
+}
+
 /// Determine structural equivalence of two records.
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
  RecordDecl *D1, RecordDecl *D2) {
-
-  // Check for equivalent structure names.
-  IdentifierInfo *Name1 = D1->getIdentifier();
-  if (!Name1 && D1->getTypedefNameForAnonDecl())
-Name1 = D1->getTypedefNameForAnonDecl()->getIdentifier();
-  IdentifierInfo *Name2 = D2->getIdentifier();
-  if (!Name2 && D2->getTypedefNameForAnonDecl())
-Name2 = D2->getTypedefNameForAnonDecl()->getIdentifier();
-  if (!IsStructurallyEquivalent(Name1, Name2))
+  if (!NameIsStructurallyEquivalent(*D1, *D2)) {
 return false;
+  }
 
   if (D1->isUnion() != D2->isUnion()) {
 if (Context.Complain) {
@@ -1727,16 +1731,9 @@
 /// Determine structural equivalence of two enums.
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
  EnumDecl *D1, EnumDecl *D2) {
-
-  // Check for equivalent enum names.
-  IdentifierInfo *Name1 = D1->getIdentifier();
-  if (!Name1 && D1->getTypedefNameForAnonDecl())
-Name1 = D1->getTypedefNameForAnonDecl()->getIdentifier();
-  IdentifierInfo *Name2 = D2->getIdentifier();
-  if (!Name2 && D2->getTypedefNameForAnonDecl())
-Name2 = D2->getTypedefNameForAnonDecl()->getIdentifier();
-  if (!IsStructurallyEquivalent(Name1, Name2))
+  if (!NameIsStructurallyEquivalent(*D1, *D2)) {
 return 

[clang] 9425271 - [clang][AST][NFC] Factor out check for structural equivalence of names.

2023-05-08 Thread Aaron Ballman via cfe-commits

Author: David Stone
Date: 2023-05-08T13:27:36-04:00
New Revision: 94252713f9a7fdeeb18a7e43a9d414ea09fc0c13

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

LOG: [clang][AST][NFC] Factor out check for structural equivalence of names.

We have four places that we try to decide which name to use for the
test for structural equivalence, and in each of those we evaluate
getTypedefNameForAnonDecl twice. Pull out the check into a function to
reduce duplication and evaluate things only once.

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

Added: 


Modified: 
clang/lib/AST/ASTStructuralEquivalence.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTStructuralEquivalence.cpp 
b/clang/lib/AST/ASTStructuralEquivalence.cpp
index ba7dfc35edf2..a9470782c634 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1453,19 +1453,23 @@ static bool 
IsRecordContextStructurallyEquivalent(RecordDecl *D1,
   return true;
 }
 
+static bool NameIsStructurallyEquivalent(const TagDecl , const TagDecl ) 
{
+  auto GetName = [](const TagDecl ) -> const IdentifierInfo * {
+if (const IdentifierInfo *Name = D.getIdentifier())
+  return Name;
+if (const TypedefNameDecl *TypedefName = D.getTypedefNameForAnonDecl())
+  return TypedefName->getIdentifier();
+return nullptr;
+  };
+  return IsStructurallyEquivalent(GetName(D1), GetName(D2));
+}
+
 /// Determine structural equivalence of two records.
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
  RecordDecl *D1, RecordDecl *D2) {
-
-  // Check for equivalent structure names.
-  IdentifierInfo *Name1 = D1->getIdentifier();
-  if (!Name1 && D1->getTypedefNameForAnonDecl())
-Name1 = D1->getTypedefNameForAnonDecl()->getIdentifier();
-  IdentifierInfo *Name2 = D2->getIdentifier();
-  if (!Name2 && D2->getTypedefNameForAnonDecl())
-Name2 = D2->getTypedefNameForAnonDecl()->getIdentifier();
-  if (!IsStructurallyEquivalent(Name1, Name2))
+  if (!NameIsStructurallyEquivalent(*D1, *D2)) {
 return false;
+  }
 
   if (D1->isUnion() != D2->isUnion()) {
 if (Context.Complain) {
@@ -1727,16 +1731,9 @@ static bool 
IsStructurallyEquivalent(StructuralEquivalenceContext ,
 /// Determine structural equivalence of two enums.
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
  EnumDecl *D1, EnumDecl *D2) {
-
-  // Check for equivalent enum names.
-  IdentifierInfo *Name1 = D1->getIdentifier();
-  if (!Name1 && D1->getTypedefNameForAnonDecl())
-Name1 = D1->getTypedefNameForAnonDecl()->getIdentifier();
-  IdentifierInfo *Name2 = D2->getIdentifier();
-  if (!Name2 && D2->getTypedefNameForAnonDecl())
-Name2 = D2->getTypedefNameForAnonDecl()->getIdentifier();
-  if (!IsStructurallyEquivalent(Name1, Name2))
+  if (!NameIsStructurallyEquivalent(*D1, *D2)) {
 return false;
+  }
 
   // Compare the definitions of these two enums. If either or both are
   // incomplete (i.e. forward declared), we assume that they are equivalent.



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


[PATCH] D149981: [clang][AST][NFC] Factor out check for structural equivalence of names.

2023-05-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM! I'll fix up the nits when I land on your behalf.




Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1456-1467
+static bool NameIsStructurallyEquivalent(const TagDecl , const TagDecl ) 
{
+  auto GetName = [](const TagDecl ) -> const IdentifierInfo * {
+if (const IdentifierInfo *Name = D.getIdentifier()) {
+  return Name;
+}
+if (const TypedefNameDecl *TypedefName = D.getTypedefNameForAnonDecl()) {
+  return TypedefName->getIdentifier();

Minor nits for our weird coding style.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149981

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


[PATCH] D149834: [clang][Interp] Fix ignoring TypeTraitExprs

2023-05-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


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

https://reviews.llvm.org/D149834

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


[PATCH] D150001: [clang] Fix initializer_list matching failures with modules

2023-05-08 Thread Alan Zhao via Phabricator via cfe-commits
ayzhao updated this revision to Diff 520429.
ayzhao marked an inline comment as done.
ayzhao added a comment.

add comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150001

Files:
  clang/include/clang/AST/DeclBase.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/Modules/match_initializer_list.cpp


Index: clang/test/Modules/match_initializer_list.cpp
===
--- /dev/null
+++ clang/test/Modules/match_initializer_list.cpp
@@ -0,0 +1,19 @@
+// RUN: rm -rf %t
+//
+// RUN: %clang_cc1 -x c++ -fmodules -fmodules-cache-path=%t \
+// RUN: -I %S/Inputs/initializer_list \
+// RUN: -fmodule-map-file=%S/Inputs/initializer_list/direct.modulemap \
+// RUN: %s -verify
+
+// expected-no-diagnostics
+
+class C {
+  public:
+  virtual ~C() {}
+};
+
+#include "Inputs/initializer_list/direct.h"
+
+void takesInitList(std::initializer_list);
+
+void passesInitList() { takesInitList({0}); }
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -11567,6 +11567,10 @@
 ().get("std"),
 /*PrevDecl=*/nullptr, /*Nested=*/false);
 getStdNamespace()->setImplicit(true);
+// We want the created NamespaceDecl to be available for redeclaration
+// lookups, but not for regular name lookups.
+Context.getTranslationUnitDecl()->addDecl(getStdNamespace());
+getStdNamespace()->clearIdentifierNamespace();
   }
 
   return getStdNamespace();
Index: clang/include/clang/AST/DeclBase.h
===
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -1172,6 +1172,12 @@
 }
   }
 
+  /// Clears the namespace of this declaration.
+  ///
+  /// This is useful if we want this declaration to be available for
+  /// redeclaration lookup but otherwise hidden for ordinary name lookups.
+  void clearIdentifierNamespace() { IdentifierNamespace = 0; }
+
   enum FriendObjectKind {
 FOK_None,  ///< Not a friend object.
 FOK_Declared,  ///< A friend of a previously-declared entity.


Index: clang/test/Modules/match_initializer_list.cpp
===
--- /dev/null
+++ clang/test/Modules/match_initializer_list.cpp
@@ -0,0 +1,19 @@
+// RUN: rm -rf %t
+//
+// RUN: %clang_cc1 -x c++ -fmodules -fmodules-cache-path=%t \
+// RUN: -I %S/Inputs/initializer_list \
+// RUN: -fmodule-map-file=%S/Inputs/initializer_list/direct.modulemap \
+// RUN: %s -verify
+
+// expected-no-diagnostics
+
+class C {
+  public:
+  virtual ~C() {}
+};
+
+#include "Inputs/initializer_list/direct.h"
+
+void takesInitList(std::initializer_list);
+
+void passesInitList() { takesInitList({0}); }
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -11567,6 +11567,10 @@
 ().get("std"),
 /*PrevDecl=*/nullptr, /*Nested=*/false);
 getStdNamespace()->setImplicit(true);
+// We want the created NamespaceDecl to be available for redeclaration
+// lookups, but not for regular name lookups.
+Context.getTranslationUnitDecl()->addDecl(getStdNamespace());
+getStdNamespace()->clearIdentifierNamespace();
   }
 
   return getStdNamespace();
Index: clang/include/clang/AST/DeclBase.h
===
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -1172,6 +1172,12 @@
 }
   }
 
+  /// Clears the namespace of this declaration.
+  ///
+  /// This is useful if we want this declaration to be available for
+  /// redeclaration lookup but otherwise hidden for ordinary name lookups.
+  void clearIdentifierNamespace() { IdentifierNamespace = 0; }
+
   enum FriendObjectKind {
 FOK_None,  ///< Not a friend object.
 FOK_Declared,  ///< A friend of a previously-declared entity.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D150071: [clang-tidy] Fix bugprone-assert-side-effect to actually give warnings

2023-05-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/assert-side-effect/assert.h:1
+int abort() { return 0; }
+

There should be no need to define the function (the real header wouldn't 
either).



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp:1
-// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- 
-config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, 
value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 
'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, {key: 
bugprone-assert-side-effect.IgnoredFunctions, value: 
'MyClass::badButIgnoredFunc'}]}" -- -fexceptions
-
-//===--- assert definition block 
--===//
-int abort() { return 0; }
-
-#ifdef NDEBUG
-#define assert(x) 1
-#else
-#define assert(x)  
\
-  if (!(x))
\
-  (void)abort()
-#endif
-
-void print(...);
-#define assert2(e) (__builtin_expect(!(e), 0) ?
\
-   print (#e, __FILE__, __LINE__) : (void)0)
-
-#ifdef NDEBUG
-#define my_assert(x) 1
-#else
-#define my_assert(x)   
\
-  ((void)((x) ? 1 : abort()))
-#endif
-
-#ifdef NDEBUG
-#define not_my_assert(x) 1
-#else
-#define not_my_assert(x)   
\
-  if (!(x))
\
-  (void)abort()
-#endif
-
-#define real_assert(x) ((void)((x) ? 1 : abort()))
-#define wrap1(x) real_assert(x)
-#define wrap2(x) wrap1(x)
-#define convoluted_assert(x) wrap2(x)
-
-#define msvc_assert(expression) (void)(
\
-(!!(expression)) ||
\
-(abort(), 0)   
\
-)
-
-
-//===--===//
+// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- 
-config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, 
value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 
'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, {key: 
bugprone-assert-side-effect.IgnoredFunctions, value: 
'MyClass::badButIgnoredFunc'}]}" -- -fexceptions -isystem 
%S/Inputs/assert-side-effect
+#include 

Perhaps a more reliable (or a "belt and suspenders") way would be to add the 
following pragma to the header:

```
#pragma clang system_header
```

This way I believe it would not matter whether the header is found through 
`-isystem` or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150071

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


[PATCH] D150038: [Clang] Improve compile times when forming a DeclRef outside of a capturing scope.

2023-05-08 Thread Corentin Jabot 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 rGa7579b25df78: [Clang] Improve compile times when forming a 
DeclRef outside of a capturing… (authored by cor3ntin).

Changed prior to commit:
  https://reviews.llvm.org/D150038?vs=520082=520420#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150038

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


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19221,6 +19221,15 @@
   // An init-capture is notionally from the context surrounding its
   // declaration, but its parent DC is the lambda class.
   DeclContext *VarDC = Var->getDeclContext();
+  DeclContext *DC = CurContext;
+
+  // tryCaptureVariable is called every time a DeclRef is formed,
+  // it can therefore have non-negigible impact on performances.
+  // For local variables and when there is no capturing scope,
+  // we can bailout early.
+  if (CapturingFunctionScopes == 0 && (!BuildAndDiagnose || VarDC == DC))
+return true;
+
   const auto *VD = dyn_cast(Var);
   if (VD) {
 if (VD->isInitCapture())
@@ -19230,7 +19239,6 @@
   }
   assert(VD && "Cannot capture a null variable");
 
-  DeclContext *DC = CurContext;
   const unsigned MaxFunctionScopesIndex = FunctionScopeIndexToStopAt
   ? *FunctionScopeIndexToStopAt : FunctionScopes.size() - 1;
   // We need to sync up the Declaration Context with the
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -2138,11 +2138,13 @@
 void Sema::PushBlockScope(Scope *BlockScope, BlockDecl *Block) {
   FunctionScopes.push_back(new BlockScopeInfo(getDiagnostics(),
   BlockScope, Block));
+  CapturingFunctionScopes++;
 }
 
 LambdaScopeInfo *Sema::PushLambdaScope() {
   LambdaScopeInfo *const LSI = new LambdaScopeInfo(getDiagnostics());
   FunctionScopes.push_back(LSI);
+  CapturingFunctionScopes++;
   return LSI;
 }
 
@@ -2264,6 +2266,8 @@
 
 void Sema::PoppedFunctionScopeDeleter::
 operator()(sema::FunctionScopeInfo *Scope) const {
+  if (!Scope->isPlainFunction())
+Self->CapturingFunctionScopes--;
   // Stash the function scope for later reuse if it's for a normal function.
   if (Scope->isPlainFunction() && !Self->CachedFunctionScope)
 Self->CachedFunctionScope.reset(Scope);
@@ -2694,6 +2698,7 @@
   OpenMPCaptureLevel);
   CSI->ReturnType = Context.VoidTy;
   FunctionScopes.push_back(CSI);
+  CapturingFunctionScopes++;
 }
 
 CapturedRegionScopeInfo *Sema::getCurCapturedRegion() {
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -806,6 +806,9 @@
   /// context.
   unsigned FunctionScopesStart = 0;
 
+  /// Track the number of currently active capturing scopes.
+  unsigned CapturingFunctionScopes = 0;
+
   ArrayRef getFunctionScopes() const {
 return llvm::ArrayRef(FunctionScopes.begin() + FunctionScopesStart,
   FunctionScopes.end());


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19221,6 +19221,15 @@
   // An init-capture is notionally from the context surrounding its
   // declaration, but its parent DC is the lambda class.
   DeclContext *VarDC = Var->getDeclContext();
+  DeclContext *DC = CurContext;
+
+  // tryCaptureVariable is called every time a DeclRef is formed,
+  // it can therefore have non-negigible impact on performances.
+  // For local variables and when there is no capturing scope,
+  // we can bailout early.
+  if (CapturingFunctionScopes == 0 && (!BuildAndDiagnose || VarDC == DC))
+return true;
+
   const auto *VD = dyn_cast(Var);
   if (VD) {
 if (VD->isInitCapture())
@@ -19230,7 +19239,6 @@
   }
   assert(VD && "Cannot capture a null variable");
 
-  DeclContext *DC = CurContext;
   const unsigned MaxFunctionScopesIndex = FunctionScopeIndexToStopAt
   ? *FunctionScopeIndexToStopAt : FunctionScopes.size() - 1;
   // We need to sync up the Declaration Context with the
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -2138,11 +2138,13 @@
 void Sema::PushBlockScope(Scope *BlockScope, BlockDecl *Block) {
   FunctionScopes.push_back(new BlockScopeInfo(getDiagnostics(),
   BlockScope, Block));
+  CapturingFunctionScopes++;
 }
 
 LambdaScopeInfo *Sema::PushLambdaScope() {
   

[clang] a7579b2 - [Clang] Improve compile times when forming a DeclRef outside of a capturing scope.

2023-05-08 Thread Corentin Jabot via cfe-commits

Author: Corentin Jabot
Date: 2023-05-08T18:41:24+02:00
New Revision: a7579b25df78a9f53d62300020d4ae3c4734

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

LOG: [Clang] Improve compile times when forming a DeclRef outside of a 
capturing scope.

The logic of whether an entity needs to be captured has become
quite complex and the recent changes in https://reviews.llvm.org/D124351
ad a mesurable negative impact on compile times.

However, in the absence of capturing scopes (lambda, block, region)
we usually can avoid running most of that logic
(except that we do need to diagnostic when a nested function
refers to a local variable in the scope of the outer function.).

This patch track whether there is currently an active capturing
scope and exit `tryCaptureVariable` early when there isn't.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaExpr.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 92f3e6f0ff848..c6facd4e9e4af 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -806,6 +806,9 @@ class Sema final {
   /// context.
   unsigned FunctionScopesStart = 0;
 
+  /// Track the number of currently active capturing scopes.
+  unsigned CapturingFunctionScopes = 0;
+
   ArrayRef getFunctionScopes() const {
 return llvm::ArrayRef(FunctionScopes.begin() + FunctionScopesStart,
   FunctionScopes.end());

diff  --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 07ece4078474e..831e9db6e4f07 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -2138,11 +2138,13 @@ void Sema::PushFunctionScope() {
 void Sema::PushBlockScope(Scope *BlockScope, BlockDecl *Block) {
   FunctionScopes.push_back(new BlockScopeInfo(getDiagnostics(),
   BlockScope, Block));
+  CapturingFunctionScopes++;
 }
 
 LambdaScopeInfo *Sema::PushLambdaScope() {
   LambdaScopeInfo *const LSI = new LambdaScopeInfo(getDiagnostics());
   FunctionScopes.push_back(LSI);
+  CapturingFunctionScopes++;
   return LSI;
 }
 
@@ -2264,6 +2266,8 @@ Sema::PopFunctionScopeInfo(const 
AnalysisBasedWarnings::Policy *WP,
 
 void Sema::PoppedFunctionScopeDeleter::
 operator()(sema::FunctionScopeInfo *Scope) const {
+  if (!Scope->isPlainFunction())
+Self->CapturingFunctionScopes--;
   // Stash the function scope for later reuse if it's for a normal function.
   if (Scope->isPlainFunction() && !Self->CachedFunctionScope)
 Self->CachedFunctionScope.reset(Scope);
@@ -2694,6 +2698,7 @@ void Sema::PushCapturedRegionScope(Scope *S, CapturedDecl 
*CD, RecordDecl *RD,
   OpenMPCaptureLevel);
   CSI->ReturnType = Context.VoidTy;
   FunctionScopes.push_back(CSI);
+  CapturingFunctionScopes++;
 }
 
 CapturedRegionScopeInfo *Sema::getCurCapturedRegion() {

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 622b36407495b..ec0f31e489d5b 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -19221,6 +19221,15 @@ bool Sema::tryCaptureVariable(
   // An init-capture is notionally from the context surrounding its
   // declaration, but its parent DC is the lambda class.
   DeclContext *VarDC = Var->getDeclContext();
+  DeclContext *DC = CurContext;
+
+  // tryCaptureVariable is called every time a DeclRef is formed,
+  // it can therefore have non-negigible impact on performances.
+  // For local variables and when there is no capturing scope,
+  // we can bailout early.
+  if (CapturingFunctionScopes == 0 && (!BuildAndDiagnose || VarDC == DC))
+return true;
+
   const auto *VD = dyn_cast(Var);
   if (VD) {
 if (VD->isInitCapture())
@@ -19230,7 +19239,6 @@ bool Sema::tryCaptureVariable(
   }
   assert(VD && "Cannot capture a null variable");
 
-  DeclContext *DC = CurContext;
   const unsigned MaxFunctionScopesIndex = FunctionScopeIndexToStopAt
   ? *FunctionScopeIndexToStopAt : FunctionScopes.size() - 1;
   // We need to sync up the Declaration Context with the



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


[PATCH] D150038: [Clang] Improve compile times when forming a DeclRef outside of a capturing scope.

2023-05-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

Fantastic, that's some nice green in there. LGTM modulo nits, please add `NFC` 
to the patch summary when landing so folks know not to expect test coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150038

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


[PATCH] D149965: [clang][Interp] Fix tests for ignored expressions

2023-05-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:593-594
 
+if (DiscardResult)
+  return true;
+

tbaeder wrote:
> aaron.ballman wrote:
> > For my own understanding -- why do we not need to call `discard()` on the 
> > operand expression?
> Is it ever legal to have an argument expr with side effects (in cases the 
> constant expression interpreter needs to handle)? That's really the only 
> reason I did it this way.
Ah, I see! No, I don't think it's possible to have an expression argument with 
side effects that's valid in a constant expression for these operators.


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

https://reviews.llvm.org/D149965

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


[PATCH] D149965: [clang][Interp] Fix tests for ignored expressions

2023-05-08 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:593-594
 
+if (DiscardResult)
+  return true;
+

aaron.ballman wrote:
> For my own understanding -- why do we not need to call `discard()` on the 
> operand expression?
Is it ever legal to have an argument expr with side effects (in cases the 
constant expression interpreter needs to handle)? That's really the only reason 
I did it this way.


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

https://reviews.llvm.org/D149965

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


[PATCH] D149965: [clang][Interp] Fix tests for ignored expressions

2023-05-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:593-594
 
+if (DiscardResult)
+  return true;
+

For my own understanding -- why do we not need to call `discard()` on the 
operand expression?



Comment at: clang/test/AST/Interp/literals.cpp:207-208
+  }
+  /// FIXME: This is rejected because the parameter so sizeof() is not 
constant.
+  ///   produce a proper diagnostic.
+  static_assert(IgnoredRejected() == 0, ""); // expected-error {{not an 
integral constant expression}} \




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

https://reviews.llvm.org/D149965

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


[PATCH] D147591: [clang][Interp] Handle CXXTemporaryObjectExprs

2023-05-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LG pending test coverage being ready to land




Comment at: clang/test/AST/Interp/records.cpp:317-318
 {
-  auto T = Test(Arr, Pos);
+  Test(Arr, Pos);
   // End of scope, should destroy Test.
 }

tbaeder wrote:
> aaron.ballman wrote:
> > tbaeder wrote:
> > > aaron.ballman wrote:
> > > > tbaeder wrote:
> > > > > aaron.ballman wrote:
> > > > > > Nit: nothing actually tests that this object is destroyed 
> > > > > > correctly. Here's an interesting test to consider:
> > > > > > ```
> > > > > > struct S {
> > > > > >   constexpr S() {}
> > > > > >   constexpr ~S() noexcept(false) { throw 12; }
> > > > > > };
> > > > > > 
> > > > > > constexpr int f() {
> > > > > >   S{};
> > > > > >   return 12;
> > > > > > }
> > > > > > 
> > > > > > static_assert(f() == 12);
> > > > > > ```
> > > > > > That should fail because `~S()` would hit the `throw` expression 
> > > > > > and thus is not valid. Note, you'll need to add 
> > > > > > `-Wno-invalid-constexpr` to your test to avoid the 
> > > > > > warning-defaults-to-error about the destructor never producing a 
> > > > > > constant expression.
> > > > > There are multiple reasons why that sample is not rejected right now, 
> > > > > one I can easily fix in a follow-up patch, the other one would 
> > > > > actually require us to recognize the `throw` and reject it with a 
> > > > > proper diagnostic.
> > > > We should definitely fix the `throw` at some point, but any of the 
> > > > dynamically reachable problematic constructs would work (`dynamic_cast` 
> > > > whose type would throw, invocation of the `va_arg` macro, 
> > > > `reinterpret_cast`, etc)
> > > Yes, I think we need a new opcode for that so we only emit the diagnostic 
> > > when such a construct is actually executed.
> > Oh yeah, you'll definitely need that, a whole pile of the constexpr rules 
> > are based around code reachability.
> > 
> > Are you saying you've got no way to test this until you implement that 
> > opcode?
> With https://reviews.llvm.org/D150040 applied, it gets properly rejected, 
> just the diagnostics are off. I can add the test and reorder the commits.
SGTM, thanks!


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

https://reviews.llvm.org/D147591

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


[clang] 8cd90fd - [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

2023-05-08 Thread Alvin Wong via cfe-commits

Author: Alvin Wong
Date: 2023-05-09T00:07:40+08:00
New Revision: 8cd90fd1a8233b2dcb96451eab7c6baea3180f54

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

LOG: [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

Clang on Windows targets often requires indirect calls through the
import address table (IAT), and also .refptr stubs for MinGW target.
On 32-bit this generates assembly in the form of
`call dword ptr [__imp__func]`, which MC had failed to handle correctly.
64-bit targets are not affected because rip-relative addressing is used.

Reported on: https://github.com/llvm/llvm-project/issues/62010

Depends on D149695, D149920

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

Added: 
llvm/test/MC/X86/intel-syntax-branch.s

Modified: 
clang/test/CodeGen/ms-inline-asm-functions.c
llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

Removed: 




diff  --git a/clang/test/CodeGen/ms-inline-asm-functions.c 
b/clang/test/CodeGen/ms-inline-asm-functions.c
index 26eaf144adf25..57abcc20ca85a 100644
--- a/clang/test/CodeGen/ms-inline-asm-functions.c
+++ b/clang/test/CodeGen/ms-inline-asm-functions.c
@@ -24,10 +24,9 @@ int foo(void) {
   __asm call kimport;
   // CHECK: calll   *({{.*}})
 
-  // Broken case: Call through a global function pointer.
+  // Call through a global function pointer.
   __asm call kptr;
-  // CHECK: calll   _kptr
-  // CHECK-FIXME: calll   *_kptr
+  // CHECK: calll   *_kptr
 }
 
 int bar(void) {

diff  --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp 
b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
index 6799234df5352..2647117f02732 100644
--- a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -433,6 +433,7 @@ class X86AsmParser : public MCTargetAsmParser {
 InlineAsmIdentifierInfo Info;
 short BracCount = 0;
 bool MemExpr = false;
+bool BracketUsed = false;
 bool OffsetOperator = false;
 bool AttachToOperandIdx = false;
 bool IsPIC = false;
@@ -455,6 +456,7 @@ class X86AsmParser : public MCTargetAsmParser {
 void addImm(int64_t imm) { Imm += imm; }
 short getBracCount() const { return BracCount; }
 bool isMemExpr() const { return MemExpr; }
+bool isBracketUsed() const { return BracketUsed; }
 bool isOffsetOperator() const { return OffsetOperator; }
 SMLoc getOffsetLoc() const { return OffsetOperatorLoc; }
 unsigned getBaseReg() const { return BaseReg; }
@@ -955,6 +957,7 @@ class X86AsmParser : public MCTargetAsmParser {
 break;
   }
   MemExpr = true;
+  BracketUsed = true;
   BracCount++;
   return false;
 }
@@ -2628,9 +2631,9 @@ bool X86AsmParser::parseIntelOperand(OperandVector 
, StringRef Name) {
   unsigned DefaultBaseReg = X86::NoRegister;
   bool MaybeDirectBranchDest = true;
 
+  bool IsUnconditionalBranch =
+  Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
   if (Parser.isParsingMasm()) {
-bool IsUnconditionalBranch =
-Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
 if (is64BitMode() && SM.getElementSize() > 0) {
   DefaultBaseReg = X86::RIP;
 }
@@ -2652,6 +2655,9 @@ bool X86AsmParser::parseIntelOperand(OperandVector 
, StringRef Name) {
 }
   }
 }
+  } else if (IsUnconditionalBranch) {
+if (PtrInOperand || SM.isBracketUsed())
+  MaybeDirectBranchDest = false;
   }
 
   if ((BaseReg || IndexReg || RegNo || DefaultBaseReg != X86::NoRegister))

diff  --git a/llvm/test/MC/X86/intel-syntax-branch.s 
b/llvm/test/MC/X86/intel-syntax-branch.s
new file mode 100644
index 0..22b91562ea51c
--- /dev/null
+++ b/llvm/test/MC/X86/intel-syntax-branch.s
@@ -0,0 +1,71 @@
+// RUN: llvm-mc -triple i686-unknown-unknown -x86-asm-syntax=intel %s | 
FileCheck %s --check-prefixes=CHECK-32,CHECK
+// RUN: llvm-mc -triple x86_64-unknown-unknown --defsym X64=1 
-x86-asm-syntax=intel %s | FileCheck %s --check-prefixes=CHECK-64,CHECK
+
+// RUN: not llvm-mc -triple i686-unknown-unknown --defsym ERR=1 
-x86-asm-syntax=intel %s 2>&1 | FileCheck %s --check-prefixes=ERR-32
+
+t0:
+call direct_branch
+jmp direct_branch
+// CHECK-LABEL: t0:
+// CHECK-64: callq direct_branch
+// CHECK-32: calll direct_branch
+// CHECK:jmp direct_branch
+
+t1:
+call [fn_ref]
+jmp [fn_ref]
+// CHECK-LABEL: t1:
+// CHECK-64: callq *fn_ref
+// CHECK-64: jmpq *fn_ref
+// CHECK-32: calll *fn_ref
+// CHECK-32: jmpl *fn_ref
+
+.ifdef X64
+
+  t2:
+  call qword ptr [fn_ref]
+  jmp qword ptr [fn_ref]
+  // CHECK-64-LABEL: t2:
+  // CHECK-64: callq *fn_ref
+  // CHECK-64: jmpq *fn_ref
+
+  t3:
+  call qword ptr [rip + fn_ref]
+  jmp qword ptr [rip + fn_ref]
+  // CHECK-64-LABEL: t3:
+  // CHECK-64: callq *fn_ref(%rip)
+  // 

[PATCH] D149579: [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

2023-05-08 Thread Alvin Wong via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8cd90fd1a823: [X86][MC] Fix parsing Intel syntax indirect 
branch with symbol only (authored by alvinhochun).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149579

Files:
  clang/test/CodeGen/ms-inline-asm-functions.c
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/test/MC/X86/intel-syntax-branch.s

Index: llvm/test/MC/X86/intel-syntax-branch.s
===
--- /dev/null
+++ llvm/test/MC/X86/intel-syntax-branch.s
@@ -0,0 +1,71 @@
+// RUN: llvm-mc -triple i686-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s --check-prefixes=CHECK-32,CHECK
+// RUN: llvm-mc -triple x86_64-unknown-unknown --defsym X64=1 -x86-asm-syntax=intel %s | FileCheck %s --check-prefixes=CHECK-64,CHECK
+
+// RUN: not llvm-mc -triple i686-unknown-unknown --defsym ERR=1 -x86-asm-syntax=intel %s 2>&1 | FileCheck %s --check-prefixes=ERR-32
+
+t0:
+call direct_branch
+jmp direct_branch
+// CHECK-LABEL: t0:
+// CHECK-64: callq direct_branch
+// CHECK-32: calll direct_branch
+// CHECK:jmp direct_branch
+
+t1:
+call [fn_ref]
+jmp [fn_ref]
+// CHECK-LABEL: t1:
+// CHECK-64: callq *fn_ref
+// CHECK-64: jmpq *fn_ref
+// CHECK-32: calll *fn_ref
+// CHECK-32: jmpl *fn_ref
+
+.ifdef X64
+
+  t2:
+  call qword ptr [fn_ref]
+  jmp qword ptr [fn_ref]
+  // CHECK-64-LABEL: t2:
+  // CHECK-64: callq *fn_ref
+  // CHECK-64: jmpq *fn_ref
+
+  t3:
+  call qword ptr [rip + fn_ref]
+  jmp qword ptr [rip + fn_ref]
+  // CHECK-64-LABEL: t3:
+  // CHECK-64: callq *fn_ref(%rip)
+  // CHECK-64: jmpq *fn_ref(%rip)
+
+.else
+
+  t4:
+  call dword ptr [fn_ref]
+  jmp dword ptr [fn_ref]
+  // CHECK-32-LABEL: t4:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+  t5:
+  call dword ptr fn_ref
+  jmp dword ptr fn_ref
+  // CHECK-32-LABEL: t5:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+  t6:
+  call dword ptr [offset fn_ref]
+  jmp dword ptr [offset fn_ref]
+  // CHECK-32-LABEL: t6:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+.ifdef ERR
+
+  call offset fn_ref
+  // ERR-32: {{.*}}.s:[[#@LINE-1]]:3: error: invalid operand for instruction
+  jmp offset fn_ref
+  // ERR-32: {{.*}}.s:[[#@LINE-1]]:3: error: invalid operand for instruction
+
+.endif
+
+.endif
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -433,6 +433,7 @@
 InlineAsmIdentifierInfo Info;
 short BracCount = 0;
 bool MemExpr = false;
+bool BracketUsed = false;
 bool OffsetOperator = false;
 bool AttachToOperandIdx = false;
 bool IsPIC = false;
@@ -455,6 +456,7 @@
 void addImm(int64_t imm) { Imm += imm; }
 short getBracCount() const { return BracCount; }
 bool isMemExpr() const { return MemExpr; }
+bool isBracketUsed() const { return BracketUsed; }
 bool isOffsetOperator() const { return OffsetOperator; }
 SMLoc getOffsetLoc() const { return OffsetOperatorLoc; }
 unsigned getBaseReg() const { return BaseReg; }
@@ -955,6 +957,7 @@
 break;
   }
   MemExpr = true;
+  BracketUsed = true;
   BracCount++;
   return false;
 }
@@ -2628,9 +2631,9 @@
   unsigned DefaultBaseReg = X86::NoRegister;
   bool MaybeDirectBranchDest = true;
 
+  bool IsUnconditionalBranch =
+  Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
   if (Parser.isParsingMasm()) {
-bool IsUnconditionalBranch =
-Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
 if (is64BitMode() && SM.getElementSize() > 0) {
   DefaultBaseReg = X86::RIP;
 }
@@ -2652,6 +2655,9 @@
 }
   }
 }
+  } else if (IsUnconditionalBranch) {
+if (PtrInOperand || SM.isBracketUsed())
+  MaybeDirectBranchDest = false;
   }
 
   if ((BaseReg || IndexReg || RegNo || DefaultBaseReg != X86::NoRegister))
Index: clang/test/CodeGen/ms-inline-asm-functions.c
===
--- clang/test/CodeGen/ms-inline-asm-functions.c
+++ clang/test/CodeGen/ms-inline-asm-functions.c
@@ -24,10 +24,9 @@
   __asm call kimport;
   // CHECK: calll   *({{.*}})
 
-  // Broken case: Call through a global function pointer.
+  // Call through a global function pointer.
   __asm call kptr;
-  // CHECK: calll   _kptr
-  // CHECK-FIXME: calll   *_kptr
+  // CHECK: calll   *_kptr
 }
 
 int bar(void) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


  1   2   3   >