[Lldb-commits] [PATCH] D101462: Make it possible for targets to define their own MCObjectFileInfo

[Philipp Krones via Phabricator via lldb-commits]

flip1995 updated this revision to Diff 342345.
flip1995 added a comment.

[MC] Untangle MCContext and MCObjectFileInfo

This untangles the MCContext and the MCObjectFileInfo. There is a circular
dependency between MCContext and MCObjectFileInfo. Currently this dependency
also exists during construction: You can't contruct a MOFI without a MCContext
without constructing the MCContext with a dummy version of that MOFI first.
This removes this dependency during construction. In a perfect world,
MCObjectFileInfo wouldn't depend on MCContext at all, but only be stored in the
MCContext, like other MC information. This is future work.

This also shifts/adds more information to the MCContext making it more
available to the different targets. Namely:

- TargetTriple
- ObjectFileType
- SubtargetInfo

Included Commits:

- [MC] Add MCSubtargetInfo to MCContext
- [MC] Untangle MCContext and MCObjectFileInfo
- [MC] Move TargetTriple information from MCObjectFileInfo to MCContext


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101462

Files:
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/tools/driver/cc1as_main.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  llvm/include/llvm/MC/MCContext.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/MachineModuleInfo.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/DWARFLinker/DWARFStreamer.cpp
  llvm/lib/MC/MCAsmStreamer.cpp
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCDisassembler/Disassembler.cpp
  llvm/lib/MC/MCMachOStreamer.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/MC/MCParser/COFFAsmParser.cpp
  llvm/lib/MC/MCParser/DarwinAsmParser.cpp
  llvm/lib/MC/MCParser/MasmParser.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/MC/MCWinCOFFStreamer.cpp
  llvm/lib/Object/ModuleSymbolTable.cpp
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXTargetStreamer.cpp
  llvm/lib/Target/TargetLoweringObjectFile.cpp
  llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-exegesis/lib/Analysis.cpp
  llvm/tools/llvm-exegesis/lib/LlvmState.cpp
  llvm/tools/llvm-exegesis/lib/SnippetFile.cpp
  llvm/tools/llvm-jitlink/llvm-jitlink.cpp
  llvm/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-ml/Disassembler.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-objdump/MachODump.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-profgen/ProfiledBinary.cpp
  llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
  llvm/tools/sancov/sancov.cpp
  llvm/unittests/CodeGen/MachineInstrTest.cpp
  llvm/unittests/CodeGen/MachineOperandTest.cpp
  llvm/unittests/CodeGen/TestAsmPrinter.cpp
  llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
  llvm/unittests/MC/DwarfLineTables.cpp
  llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp

Index: llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
===
--- llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
+++ llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
@@ -57,6 +57,7 @@
   std::unique_ptr MRI;
   std::unique_ptr MUPMAI;
   std::unique_ptr MII;
+  std::unique_ptr MSTI;
   std::unique_ptr Str;
   std::unique_ptr Parser;
   std::unique_ptr Ctx;
@@ -86,6 +87,11 @@
 MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
 EXPECT_NE(MAI, nullptr);
 
+std::unique_ptr MSTI;
+MSTI.reset(TheTarget->createMCSubtargetInfo(TripleName, /*CPU=*/"",
+/*Features=*/""));
+EXPECT_NE(MSTI, nullptr);
+
 // Now we cast to our mocked up version of MCAsmInfo.
 MUPMAI.reset(static_cast(MAI.release()));
 // MUPMAI should "hold" MAI.
@@ -99,9 +105,9 @@
 SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
 EXPECT_EQ(Buffer, nullptr);
 
-Ctx.reset(
-new MCContext(MUPMAI.get(), MRI.get(), &MOFI, &SrcMgr, &MCOptions));
-MOFI.InitMCObjectFileInfo(Triple, false, *Ctx, false);
+Ctx.reset(new MCContext(Triple, MUPMAI.get(), MRI.get(), &MOFI, MSTI.get(),
+&SrcMgr, &MCOptions));
+MOFI.InitMCObjectFileInfo(/*PIC=*/false, *Ctx, /*LargeCodeModel=*/false);
 
 Str.reset(TheTarget->createNullStreamer(*Ctx));
 
Index: llvm/unittests/MC/DwarfLineTables.cpp
===
--- llvm/unittests/MC/DwarfLineTables.cpp
+++ llvm/unittests/MC/DwarfLineTables.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 struct Context {
-  const char *Triple = "x86_64-pc-linux";
+  const char *TripleName = "x86_64-pc-linux";
   std::unique_ptr 

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

[Fangrui Song via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG632ebc4ab437: [MC] Untangle MCContext and MCObjectFileInfo 
(authored by flip1995, committed by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101462

Files:
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/tools/driver/cc1as_main.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  llvm/include/llvm/MC/MCContext.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/MachineModuleInfo.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/DWARFLinker/DWARFStreamer.cpp
  llvm/lib/MC/MCAsmStreamer.cpp
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCDisassembler/Disassembler.cpp
  llvm/lib/MC/MCMachOStreamer.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/MC/MCParser/COFFAsmParser.cpp
  llvm/lib/MC/MCParser/DarwinAsmParser.cpp
  llvm/lib/MC/MCParser/MasmParser.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/MC/MCWinCOFFStreamer.cpp
  llvm/lib/Object/ModuleSymbolTable.cpp
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXTargetStreamer.cpp
  llvm/lib/Target/TargetLoweringObjectFile.cpp
  llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-exegesis/lib/Analysis.cpp
  llvm/tools/llvm-exegesis/lib/LlvmState.cpp
  llvm/tools/llvm-exegesis/lib/SnippetFile.cpp
  llvm/tools/llvm-jitlink/llvm-jitlink.cpp
  llvm/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-ml/Disassembler.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-objdump/MachODump.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-profgen/ProfiledBinary.cpp
  llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
  llvm/tools/sancov/sancov.cpp
  llvm/unittests/CodeGen/MachineInstrTest.cpp
  llvm/unittests/CodeGen/MachineOperandTest.cpp
  llvm/unittests/CodeGen/TestAsmPrinter.cpp
  llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
  llvm/unittests/MC/DwarfLineTables.cpp
  llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
  mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp

Index: mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
===
--- mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
+++ mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
@@ -169,8 +169,8 @@
   mai->setRelaxELFRelocations(true);
 
   llvm::MCObjectFileInfo mofi;
-  llvm::MCContext ctx(mai.get(), mri.get(), &mofi, &srcMgr, &mcOptions);
-  mofi.InitMCObjectFileInfo(triple, false, ctx, false);
+  llvm::MCContext ctx(triple, mai.get(), mri.get(), &mofi, &srcMgr, &mcOptions);
+  mofi.initMCObjectFileInfo(ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
 
   SmallString<128> cwd;
   if (!llvm::sys::fs::current_path(cwd))
Index: llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
===
--- llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
+++ llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
@@ -112,9 +112,9 @@
 SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
 EXPECT_EQ(Buffer, nullptr);
 
-Ctx.reset(
-new MCContext(MUPMAI.get(), MRI.get(), &MOFI, &SrcMgr, &MCOptions));
-MOFI.InitMCObjectFileInfo(Triple, false, *Ctx, false);
+Ctx.reset(new MCContext(Triple, MUPMAI.get(), MRI.get(), &MOFI, STI.get(),
+&SrcMgr, &MCOptions));
+MOFI.initMCObjectFileInfo(*Ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
 
 Str.reset(TheTarget->createNullStreamer(*Ctx));
 
Index: llvm/unittests/MC/DwarfLineTables.cpp
===
--- llvm/unittests/MC/DwarfLineTables.cpp
+++ llvm/unittests/MC/DwarfLineTables.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 struct Context {
-  const char *Triple = "x86_64-pc-linux";
+  const char *TripleName = "x86_64-pc-linux";
   std::unique_ptr MRI;
   std::unique_ptr MAI;
   std::unique_ptr Ctx;
@@ -33,14 +33,15 @@
 
 // If we didn't build x86, do not run the test.
 std::string Error;
-const Target *TheTarget = TargetRegistry::lookupTarget(Triple, Error);
+const Target *TheTarget = TargetRegistry::lookupTarget(TripleName, Error);
 if (!TheTarget)
   return;
 
-MRI.reset(TheTarget->createMCRegInfo(Triple));
+MRI.reset(TheTarget->createMCRegInfo(TripleName));
 MCTargetOptions MCOptions;
-MAI.reset(TheTarget->createMCAsmInfo(*MRI, Triple, MCOptions));
-Ctx = std::make_unique(MAI.get(), MRI.get(), nullptr);
+MAI.reset(TheTarg

[Lldb-commits] [PATCH] D101702: [clang-format] Add more support for C# 8 nullables

[Eliza via Phabricator via lldb-commits]

exv added a comment.

Hey all, I'm really sorry for the noise, I screwed up my arc command to revise 
the submission.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101702

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

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

[Fangrui Song via Phabricator via lldb-commits]

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

Looks great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101462

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

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

[Philipp Krones via Phabricator via lldb-commits]

flip1995 added a comment.

Not sure how the process from here on out is. I think it is important to note, 
that I don't have push rights and someone else will have to land this for me (I 
guess?).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101462

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

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

In D101462#2733726 , @flip1995 wrote:

>> I'll keep this open for a few days as it touches too many things.
>
> Sounds good 👍
>
>> but you'll need to provide your name and email
>
> ~~I used `arc diff`. The commits I made with `git` have my name and email 
> attached. But it seems like `arc` doesn't use them? I'll figure it out 
> tomorrow, can't be that hard, I hope.~~
>
> Ah you just need that to commit it. In that case: "Philipp Krones 
> "

Pushed:) You are right. A diff applied with `arc diff` can be fetched locally 
with `arc patch D101462` with the author information.
(Sometimes `arc patch` fail refuse to apply and then I'll have no idea how to 
get the author information.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101462

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

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

[Jessica Clarke via Phabricator via lldb-commits]

jrtc27 added a comment.

In D101462#2739404 , @MaskRay wrote:

> In D101462#2733726 , @flip1995 
> wrote:
>
>>> I'll keep this open for a few days as it touches too many things.
>>
>> Sounds good 👍
>>
>>> but you'll need to provide your name and email
>>
>> ~~I used `arc diff`. The commits I made with `git` have my name and email 
>> attached. But it seems like `arc` doesn't use them? I'll figure it out 
>> tomorrow, can't be that hard, I hope.~~
>>
>> Ah you just need that to commit it. In that case: "Philipp Krones 
>> "
>
> Pushed:) You are right. A diff applied with `arc diff` can be fetched locally 
> with `arc patch D101462` with the author information.
> (Sometimes `arc patch` fail refuse to apply and then I'll have no idea how to 
> get the author information.)

`arc amend` lets you replace the metadata with the same thing `arc patch` adds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101462

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

[Lldb-commits] [PATCH] D100810: Use `GNUInstallDirs` to support custom installation dirs. -- LLVM

[John Ericson via Phabricator via lldb-commits]

Ericson2314 added a subscriber: compnerd.
Ericson2314 added a comment.

@compnerd Would you like to review this one too (which the other you approved 
needs)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100810

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

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

[Philipp Krones via Phabricator via lldb-commits]

flip1995 added a comment.

> I'll keep this open for a few days as it touches too many things.

Sounds good 👍

I used `arc diff`. The commits I made with `git` have my name and email 
attached. But it seems like `arc` doesn't use them? I'll figure it out 
tomorrow, can't be that hard, I hope.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101462

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

[Lldb-commits] [PATCH] D101702: [clang-format] Add more support for C# 8 nullables

[Eliza via Phabricator via lldb-commits]

exv updated this revision to Diff 342521.
exv added a comment.
Herald added a subscriber: JDevlieghere.

Fixing arc, hopefully?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101702

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/FormatTokenLexer.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestCSharp.cpp

Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -358,6 +358,29 @@
   verifyFormat("return _name ?? \"DEF\";");
 }
 
+TEST_F(FormatTestCSharp, CSharpNullCoalescingAssignment) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.SpaceBeforeAssignmentOperators = true;
+
+  verifyFormat("test \?\?= ABC;", Style);
+  verifyFormat("test \?\?= true;", Style);
+
+  Style.SpaceBeforeAssignmentOperators = false;
+
+  verifyFormat("test\?\?= ABC;", Style);
+  verifyFormat("test\?\?= true;", Style);
+}
+
+TEST_F(FormatTestCSharp, CSharpNullForgiving) {
+  verifyFormat("var test = null!;");
+  verifyFormat("string test = someFunctionCall()! + \"ABC\"!");
+  verifyFormat("int test = (1! + 2 + bar! + foo())!");
+  verifyFormat("test \?\?= !foo!;");
+  verifyFormat("test = !bar! \?\? !foo!;");
+  verifyFormat("bool test = !(!true || !null! || !false && !false! && !bar()! "
+   "+ (!foo()))!");
+}
+
 TEST_F(FormatTestCSharp, AttributesIndentation) {
   FormatStyle Style = getMicrosoftStyle(FormatStyle::LK_CSharp);
   Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1579,24 +1579,29 @@
   }
 }
 
-if (Style.Language == FormatStyle::LK_JavaScript) {
-  if (Current.is(tok::exclaim)) {
-if (Current.Previous &&
-(Keywords.IsJavaScriptIdentifier(
- *Current.Previous, /* AcceptIdentifierName= */ true) ||
- Current.Previous->isOneOf(
- tok::kw_namespace, tok::r_paren, tok::r_square, tok::r_brace,
- Keywords.kw_type, Keywords.kw_get, Keywords.kw_set) ||
- Current.Previous->Tok.isLiteral())) {
-  Current.setType(TT_JsNonNullAssertion);
-  return;
-}
-if (Current.Next &&
-Current.Next->isOneOf(TT_BinaryOperator, Keywords.kw_as)) {
+if ((Style.Language == FormatStyle::LK_JavaScript || Style.isCSharp()) &&
+Current.is(tok::exclaim)) {
+  if (Current.Previous) {
+bool isIdentifier =
+Style.Language == FormatStyle::LK_JavaScript
+? Keywords.IsJavaScriptIdentifier(
+  *Current.Previous, /* AcceptIdentifierName= */ true)
+: Current.Previous->is(tok::identifier);
+if (isIdentifier ||
+Current.Previous->isOneOf(
+tok::kw_namespace, tok::r_paren, tok::r_square, tok::r_brace,
+tok::kw_false, tok::kw_true, Keywords.kw_type, Keywords.kw_get,
+Keywords.kw_set) ||
+Current.Previous->Tok.isLiteral()) {
   Current.setType(TT_JsNonNullAssertion);
   return;
 }
   }
+  if (Current.Next &&
+  Current.Next->isOneOf(TT_BinaryOperator, Keywords.kw_as)) {
+Current.setType(TT_JsNonNullAssertion);
+return;
+  }
 }
 
 // Line.MightBeFunctionDecl can only be true after the parentheses of a
@@ -3186,6 +3191,10 @@
 if (Left.is(TT_CSharpNullCoalescing) || Right.is(TT_CSharpNullCoalescing))
   return true;
 
+// No space before null forgiving '!'.
+if (Right.is(TT_JsNonNullAssertion))
+  return false;
+
 // No space before '?['.
 if (Right.is(TT_CSharpNullConditionalLSquare))
   return false;
Index: clang/lib/Format/FormatTokenLexer.h
===
--- clang/lib/Format/FormatTokenLexer.h
+++ clang/lib/Format/FormatTokenLexer.h
@@ -54,7 +54,8 @@
   bool tryMergeJSPrivateIdentifier();
   bool tryMergeCSharpStringLiteral();
   bool tryMergeCSharpKeywordVariables();
-  bool tryMergeCSharpDoubleQuestion();
+  bool tryMergeCSharpNullCoalescing();
+  bool tryMergeCSharpNullCoalescingAssignment();
   bool tryMergeCSharpNullConditional();
   bool tryTransformCSharpForEach();
   bool tryMergeForEach();
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -97,7 +97,9 @@
   return;
 if (tryMergeCSharpStringLiteral())
   return;
-if (tryMergeCSharpDoubleQuestion())
+if (tryMergeCSharpN

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

In D101462#2733691 , @flip1995 wrote:

> Not sure how the process from here on out is. I think it is important to 
> note, that I don't have push rights and someone else will have to land this 
> for me (I guess?).

I'll keep this open for a few days as it touches too many things.
I can push it on your behalf afterwards but you'll need to provide your name 
and email 
https://llvm.org/docs/Phabricator.html#committing-someone-s-change-from-phabricator
(If you uploaded the diff with with `git format-patch -1` instead of `git diff` 
the information would have been available)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101462

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

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

[Anirudh Prasad via Phabricator via lldb-commits]

anirudhp added a comment.

Hi @flip1995
My apologies, but you might have to rebase this on the latest main. I 
introduced a `MCSubtargetInfo` field in https://reviews.llvm.org/D100975. Some 
of the existing changes to the file may not be needed anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101462

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

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

[Philipp Krones via Phabricator via lldb-commits]

flip1995 updated this revision to Diff 342352.
flip1995 marked 3 inline comments as done.
flip1995 edited the summary of this revision.
flip1995 added a comment.
Herald added subscribers: dcaballe, cota, teijeong, rdzhabarov, tatianashp, 
msifontes, jurahul, Kayjukh, grosul1, Joonsoo, stephenneuendorffer, liufengdb, 
aartbik, lucyrfox, mgester, arpith-jacob, csigg, nicolasvasilache, antiagainst, 
shauheen, rriddle, mehdi_amini.
Herald added a reviewer: herhut.
Herald added a project: MLIR.

Rename InitMCObjectFileInfo and reorder arguments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101462

Files:
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/tools/driver/cc1as_main.cpp
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/DWARFLinker/DWARFStreamer.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/Object/ModuleSymbolTable.cpp
  llvm/lib/Target/TargetLoweringObjectFile.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-exegesis/lib/SnippetFile.cpp
  llvm/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-profgen/ProfiledBinary.cpp
  llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
  mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp

Index: mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
===
--- mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
+++ mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
@@ -169,8 +169,8 @@
   mai->setRelaxELFRelocations(true);
 
   llvm::MCObjectFileInfo mofi;
-  llvm::MCContext ctx(mai.get(), mri.get(), &mofi, &srcMgr, &mcOptions);
-  mofi.InitMCObjectFileInfo(triple, false, ctx, false);
+  llvm::MCContext ctx(triple, mai.get(), mri.get(), &mofi, &srcMgr, &mcOptions);
+  mofi.initMCObjectFileInfo(ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
 
   SmallString<128> cwd;
   if (!llvm::sys::fs::current_path(cwd))
Index: llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
===
--- llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
+++ llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
@@ -107,7 +107,7 @@
 
 Ctx.reset(new MCContext(Triple, MUPMAI.get(), MRI.get(), &MOFI, MSTI.get(),
 &SrcMgr, &MCOptions));
-MOFI.InitMCObjectFileInfo(/*PIC=*/false, *Ctx, /*LargeCodeModel=*/false);
+MOFI.initMCObjectFileInfo(*Ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
 
 Str.reset(TheTarget->createNullStreamer(*Ctx));
 
Index: llvm/tools/llvm-profgen/ProfiledBinary.cpp
===
--- llvm/tools/llvm-profgen/ProfiledBinary.cpp
+++ llvm/tools/llvm-profgen/ProfiledBinary.cpp
@@ -334,7 +334,7 @@
 
   MCObjectFileInfo MOFI;
   MCContext Ctx(Triple(TripleName), AsmInfo.get(), MRI.get(), &MOFI, STI.get());
-  MOFI.InitMCObjectFileInfo(/*PIC=*/false, Ctx);
+  MOFI.initMCObjectFileInfo(Ctx, /*PIC=*/false);
   DisAsm.reset(TheTarget->createMCDisassembler(*STI, Ctx));
   if (!DisAsm)
 exitWithError("no disassembler for target " + TripleName, FileName);
Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -1579,7 +1579,7 @@
   MCObjectFileInfo MOFI;
   MCContext Ctx(Triple(TripleName), AsmInfo.get(), MRI.get(), &MOFI, STI.get());
   // FIXME: for now initialize MCObjectFileInfo with default values
-  MOFI.InitMCObjectFileInfo(/*PIC=*/false, Ctx);
+  MOFI.initMCObjectFileInfo(Ctx, /*PIC=*/false);
 
   std::unique_ptr DisAsm(
   TheTarget->createMCDisassembler(*STI, Ctx));
Index: llvm/tools/llvm-ml/llvm-ml.cpp
===
--- llvm/tools/llvm-ml/llvm-ml.cpp
+++ llvm/tools/llvm-ml/llvm-ml.cpp
@@ -283,7 +283,7 @@
   // MCObjectFileInfo needs a MCContext reference in order to initialize itself.
   MCObjectFileInfo MOFI;
   MCContext Ctx(TheTriple, MAI.get(), MRI.get(), &MOFI, STI.get(), &SrcMgr);
-  MOFI.InitMCObjectFileInfo(/*PIC=*/false, Ctx,
+  MOFI.initMCObjectFileInfo(Ctx, /*PIC=*/false,
 /*LargeCodeModel=*/true);
 
   if (InputArgs.hasArg(OPT_save_temp_labels))
Index: llvm/tools/llvm-mca/llvm-mca.cpp
===
--- llvm/tools/llvm-mca/llvm-mca.cpp
+++ llvm/tools/llvm-mca/llvm-mca.cpp
@@ -379,7 +379,7 @@
 
   MCContext Ctx(TheTriple, MAI.get(), MRI.get(), &MOFI, STI.get(), &SrcMgr);
 
-  MOFI.InitMCObjectFileInfo(/*PIC=*/false, Ctx);
+  MOFI.initMCObjectFileInfo(Ctx, /*PIC=*/false);
 
   std::unique_ptr BOS;
 
Index: llvm/tools/llvm-mc/llvm-mc.cpp
===
--- llvm

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

[Philipp Krones via Phabricator via lldb-commits]

flip1995 updated this revision to Diff 343056.
flip1995 added a comment.

rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101462

Files:
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/tools/driver/cc1as_main.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  llvm/include/llvm/MC/MCContext.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/MachineModuleInfo.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/DWARFLinker/DWARFStreamer.cpp
  llvm/lib/MC/MCAsmStreamer.cpp
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCDisassembler/Disassembler.cpp
  llvm/lib/MC/MCMachOStreamer.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/MC/MCParser/COFFAsmParser.cpp
  llvm/lib/MC/MCParser/DarwinAsmParser.cpp
  llvm/lib/MC/MCParser/MasmParser.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/MC/MCWinCOFFStreamer.cpp
  llvm/lib/Object/ModuleSymbolTable.cpp
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXTargetStreamer.cpp
  llvm/lib/Target/TargetLoweringObjectFile.cpp
  llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-exegesis/lib/Analysis.cpp
  llvm/tools/llvm-exegesis/lib/LlvmState.cpp
  llvm/tools/llvm-exegesis/lib/SnippetFile.cpp
  llvm/tools/llvm-jitlink/llvm-jitlink.cpp
  llvm/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-ml/Disassembler.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-objdump/MachODump.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-profgen/ProfiledBinary.cpp
  llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
  llvm/tools/sancov/sancov.cpp
  llvm/unittests/CodeGen/MachineInstrTest.cpp
  llvm/unittests/CodeGen/MachineOperandTest.cpp
  llvm/unittests/CodeGen/TestAsmPrinter.cpp
  llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
  llvm/unittests/MC/DwarfLineTables.cpp
  llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
  mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp

Index: mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
===
--- mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
+++ mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
@@ -169,8 +169,8 @@
   mai->setRelaxELFRelocations(true);
 
   llvm::MCObjectFileInfo mofi;
-  llvm::MCContext ctx(mai.get(), mri.get(), &mofi, &srcMgr, &mcOptions);
-  mofi.InitMCObjectFileInfo(triple, false, ctx, false);
+  llvm::MCContext ctx(triple, mai.get(), mri.get(), &mofi, &srcMgr, &mcOptions);
+  mofi.initMCObjectFileInfo(ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
 
   SmallString<128> cwd;
   if (!llvm::sys::fs::current_path(cwd))
Index: llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
===
--- llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
+++ llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
@@ -112,9 +112,9 @@
 SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
 EXPECT_EQ(Buffer, nullptr);
 
-Ctx.reset(
-new MCContext(MUPMAI.get(), MRI.get(), &MOFI, &SrcMgr, &MCOptions));
-MOFI.InitMCObjectFileInfo(Triple, false, *Ctx, false);
+Ctx.reset(new MCContext(Triple, MUPMAI.get(), MRI.get(), &MOFI, STI.get(),
+&SrcMgr, &MCOptions));
+MOFI.initMCObjectFileInfo(*Ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
 
 Str.reset(TheTarget->createNullStreamer(*Ctx));
 
Index: llvm/unittests/MC/DwarfLineTables.cpp
===
--- llvm/unittests/MC/DwarfLineTables.cpp
+++ llvm/unittests/MC/DwarfLineTables.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 struct Context {
-  const char *Triple = "x86_64-pc-linux";
+  const char *TripleName = "x86_64-pc-linux";
   std::unique_ptr MRI;
   std::unique_ptr MAI;
   std::unique_ptr Ctx;
@@ -33,14 +33,15 @@
 
 // If we didn't build x86, do not run the test.
 std::string Error;
-const Target *TheTarget = TargetRegistry::lookupTarget(Triple, Error);
+const Target *TheTarget = TargetRegistry::lookupTarget(TripleName, Error);
 if (!TheTarget)
   return;
 
-MRI.reset(TheTarget->createMCRegInfo(Triple));
+MRI.reset(TheTarget->createMCRegInfo(TripleName));
 MCTargetOptions MCOptions;
-MAI.reset(TheTarget->createMCAsmInfo(*MRI, Triple, MCOptions));
-Ctx = std::make_unique(MAI.get(), MRI.get(), nullptr);
+MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
+Ctx = std::make_unique(Triple(TripleName), MAI.get(), MRI.get(),
+  /*MOFI=*/nul

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

[Philipp Krones via Phabricator via lldb-commits]

flip1995 updated this revision to Diff 342354.
flip1995 added a comment.

Fix arc mistake...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101462

Files:
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/tools/driver/cc1as_main.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  llvm/include/llvm/MC/MCContext.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/MachineModuleInfo.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/DWARFLinker/DWARFStreamer.cpp
  llvm/lib/MC/MCAsmStreamer.cpp
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCDisassembler/Disassembler.cpp
  llvm/lib/MC/MCMachOStreamer.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/MC/MCParser/COFFAsmParser.cpp
  llvm/lib/MC/MCParser/DarwinAsmParser.cpp
  llvm/lib/MC/MCParser/MasmParser.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/MC/MCWinCOFFStreamer.cpp
  llvm/lib/Object/ModuleSymbolTable.cpp
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXTargetStreamer.cpp
  llvm/lib/Target/TargetLoweringObjectFile.cpp
  llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-exegesis/lib/Analysis.cpp
  llvm/tools/llvm-exegesis/lib/LlvmState.cpp
  llvm/tools/llvm-exegesis/lib/SnippetFile.cpp
  llvm/tools/llvm-jitlink/llvm-jitlink.cpp
  llvm/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-ml/Disassembler.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-objdump/MachODump.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-profgen/ProfiledBinary.cpp
  llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
  llvm/tools/sancov/sancov.cpp
  llvm/unittests/CodeGen/MachineInstrTest.cpp
  llvm/unittests/CodeGen/MachineOperandTest.cpp
  llvm/unittests/CodeGen/TestAsmPrinter.cpp
  llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
  llvm/unittests/MC/DwarfLineTables.cpp
  llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
  mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp

Index: mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
===
--- mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
+++ mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
@@ -169,8 +169,8 @@
   mai->setRelaxELFRelocations(true);
 
   llvm::MCObjectFileInfo mofi;
-  llvm::MCContext ctx(mai.get(), mri.get(), &mofi, &srcMgr, &mcOptions);
-  mofi.InitMCObjectFileInfo(triple, false, ctx, false);
+  llvm::MCContext ctx(triple, mai.get(), mri.get(), &mofi, &srcMgr, &mcOptions);
+  mofi.initMCObjectFileInfo(ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
 
   SmallString<128> cwd;
   if (!llvm::sys::fs::current_path(cwd))
Index: llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
===
--- llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
+++ llvm/unittests/MC/SystemZ/SystemZAsmLexerTest.cpp
@@ -57,6 +57,7 @@
   std::unique_ptr MRI;
   std::unique_ptr MUPMAI;
   std::unique_ptr MII;
+  std::unique_ptr MSTI;
   std::unique_ptr Str;
   std::unique_ptr Parser;
   std::unique_ptr Ctx;
@@ -86,6 +87,11 @@
 MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
 EXPECT_NE(MAI, nullptr);
 
+std::unique_ptr MSTI;
+MSTI.reset(TheTarget->createMCSubtargetInfo(TripleName, /*CPU=*/"",
+/*Features=*/""));
+EXPECT_NE(MSTI, nullptr);
+
 // Now we cast to our mocked up version of MCAsmInfo.
 MUPMAI.reset(static_cast(MAI.release()));
 // MUPMAI should "hold" MAI.
@@ -99,9 +105,9 @@
 SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
 EXPECT_EQ(Buffer, nullptr);
 
-Ctx.reset(
-new MCContext(MUPMAI.get(), MRI.get(), &MOFI, &SrcMgr, &MCOptions));
-MOFI.InitMCObjectFileInfo(Triple, false, *Ctx, false);
+Ctx.reset(new MCContext(Triple, MUPMAI.get(), MRI.get(), &MOFI, MSTI.get(),
+&SrcMgr, &MCOptions));
+MOFI.initMCObjectFileInfo(*Ctx, /*PIC=*/false, /*LargeCodeModel=*/false);
 
 Str.reset(TheTarget->createNullStreamer(*Ctx));
 
Index: llvm/unittests/MC/DwarfLineTables.cpp
===
--- llvm/unittests/MC/DwarfLineTables.cpp
+++ llvm/unittests/MC/DwarfLineTables.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 struct Context {
-  const char *Triple = "x86_64-pc-linux";
+  const char *TripleName = "x86_64-pc-linux";
   std::unique_ptr MRI;
   std::unique_ptr MAI;
   std::unique_ptr Ctx;
@@ -33,14 +33,15 @@
 
 // If we didn't build x86, do not run the test.
 std::st

[Lldb-commits] [PATCH] D101702: [clang-format] Add more support for C# 8 nullables

[Eliza via Phabricator via lldb-commits]

exv updated this revision to Diff 342520.
exv added a comment.
Herald added subscribers: llvm-commits, libcxx-commits, lldb-commits, 
Sanitizers, dcaballe, cota, teijeong, frasercrmck, dexonsmith, rdzhabarov, 
tatianashp, lxfind, dang, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, 
stephenneuendorffer, kerbowa, liufengdb, aartbik, lucyrfox, mgester, 
arpith-jacob, csigg, nicolasvasilache, antiagainst, shauheen, rriddle, 
mehdi_amini, luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, 
jocewei, PkmX, jfb, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, 
zzheng, MaskRay, jrtc27, gbedwell, niosHD, cryptoad, sabuasal, simoncook, 
johnrusso, rbar, asb, javed.absar, kbarton, hiraditya, mgorny, nhaehnle, 
jvesely, nemanjai, emaste, jholewinski.
Herald added a reviewer: andreadb.
Herald added a reviewer: rriddle.
Herald added a reviewer: aartbik.
Herald added a reviewer: aartbik.
Herald added projects: Sanitizers, LLDB, libc++, MLIR, LLVM, clang-tools-extra.
Herald added a reviewer: libc++.

Fix incorrect arc usage


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101702

Files:
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/AST/TemplateBase.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/FormatTokenLexer.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vcompress.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vcompress.c
  clang/test/CodeGen/builtins-ppc-altivec.c
  clang/test/CodeGen/builtins-ppc-vsx.c
  clang/test/CodeGenCXX/debug-info-codeview-display-name.cpp
  clang/test/CodeGenOpenCL/amdgpu-ieee.cl
  clang/test/Driver/darwin-ld-platform-version-macos.c
  clang/test/SemaTemplate/temp_arg_nontype_cxx11.cpp
  clang/unittests/Format/FormatTestCSharp.cpp
  compiler-rt/lib/scudo/standalone/combined.h
  compiler-rt/lib/scudo/standalone/internal_defs.h
  compiler-rt/lib/scudo/standalone/list.h
  compiler-rt/lib/scudo/standalone/local_cache.h
  compiler-rt/lib/scudo/standalone/mutex.h
  compiler-rt/lib/scudo/standalone/options.h
  compiler-rt/lib/scudo/standalone/primary32.h
  compiler-rt/lib/scudo/standalone/primary64.h
  compiler-rt/lib/scudo/standalone/quarantine.h
  compiler-rt/lib/scudo/standalone/secondary.h
  compiler-rt/lib/scudo/standalone/stack_depot.h
  compiler-rt/lib/scudo/standalone/stats.h
  compiler-rt/lib/scudo/standalone/tsd.h
  compiler-rt/lib/scudo/standalone/tsd_exclusive.h
  compiler-rt/lib/scudo/standalone/wrappers_c.cpp
  compiler-rt/lib/scudo/standalone/wrappers_c_bionic.cpp
  libcxx/include/__config
  libcxx/include/__iterator/concepts.h
  libcxx/include/span
  
libcxx/test/libcxx/iterators/iterator.concepts/iterator.concept.input/subsumption.compile.pass.cpp
  
libcxx/test/libcxx/iterators/iterator.requirements/iterator.assoc.types/iterator.traits/legacy_bidirectional_iterator.compile.pass.cpp
  
libcxx/test/libcxx/iterators/iterator.requirements/iterator.assoc.types/iterator.traits/legacy_forward_iterator.compile.pass.cpp
  
libcxx/test/libcxx/iterators/iterator.requirements/iterator.assoc.types/iterator.traits/legacy_input_iterator.compile.pass.cpp
  
libcxx/test/libcxx/iterators/iterator.requirements/iterator.assoc.types/iterator.traits/legacy_iterator.compile.pass.cpp
  
libcxx/test/libcxx/iterators/iterator.requirements/iterator.assoc.types/iterator.traits/legacy_random_access_iterator.compile.pass.cpp
  
libcxx/test/libcxx/iterators/iterator.requirements/iterator.assoc.types/iterator.traits/locale_dependent.compile.pass.cpp
  
libcxx/test/libcxx/iterators/iterator.requirements/iterator.concepts/integer_like.compile.pass.cpp
  
libcxx/test/std/containers/associative/map/iterator_concept_conformance.compile.pass.cpp
  
libcxx/test/std/containers/associative/map/range_concept_conformance.compile.pass.cpp
  
libcxx/test/std/containers/associative/multimap/iterator_concept_conformance.compile.pass.cpp
  
libcxx/test/std/containers/associative/multimap/range_concept_conformance.compile.pass.cpp
  
libcxx/test/std/containers/associative/multiset/iterator_concept_conformance.compile.pass.cpp
  
libcxx/test/std/containers/associative/multiset/range_concept_conformance.compile.pass.cpp
  
libcxx/test/std/containers/associative/set/iterator_concept_conformance.compile.pass.cpp
  
libcxx/test/std/containers/associative/set/range_concept_conformance.compile.pass.cpp
  
libcxx/test/std/containers/sequences/array/iterator_concept_conformance.compile.pass.cpp
  
libcxx/test/std/containers/sequences/array/range_concept_conformance.compile.pass.cpp
  
libcxx/test/std/containers/sequences/deque/it

[Lldb-commits] [PATCH] D102085: Add an "interrupt timeout" to Process, fix a bug in handling interrupt timeouts in

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: clayborg, labath, JDevlieghere.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch started as an attempt to rationalize the handling of the timeout for 
interrupting a running process.  Process::Halt sends a 10 second timeout to 
WaitForProcessToStop, but that timeout didn't have any effect on how the 
interrupt was actually handled in ProcessGDBRemote & its GDBRemoteClient 
client, and there was no way to plumb the desired timeout to that layer.

This patch adds an interrupt-timeout setting to process and plumbs that through.

That then allowed me to write some more tests on the interrupt handling, in the 
course of which I found a bug in the handling of the interrupt timeout.

The lowest level waiting on the remote stub doesn't block when the process is 
running.  Instead, it passes in a 5 second wakeup timeout when it calls 
ReadPacket.  That's there so we can detect if the connection has dropped, but 
in the current code it is also the de facto interrupt timeout as well.

When ProcessGDBRemote wants to interrupt a running process, it sends a break to 
debugserver, and sets some ivars that say "interrupt is in flight" and to 
provide the "interrupt timepoint" to the client.

When the GDBRemoteClientBase object wakes up from ReadPacket because of  the 
timeout, it checks whether there was an interrupt in flight, and if so, next 
checks whether 5 seconds have elapsed since the interrupt request.  If that 
latter test is NOT true, it is supposed to go back into ReadPacket to wait out 
the remaining time (*).  But it flubs this part.   Instead of calling 
"continue" to go back to top of the ReadPacket loop, it just breaks from the 
"result checking" switch.  The next thing after the switch is a check to see if 
the response packet is empty, and if it is empty, we exit from the ReadPacket 
loop, returning eStateInvalid - the same state as if the interrupt had failed.

But if we woke up before the interrupt had a chance to take effect, there won't 
be any response yet, and so we error out.  That looks counter to the intention 
of the code that was checking the timeouts.  It's bad because it means that if 
you are unlucky and happened to request the interrupt just before the 
ReadPacket call exceeds the timeout, you could in fact end up waiting only a 
very short time for the interrupt.

I have lots of reports of this happening, but never reproducibly.  The reports 
always come in contexts where lots of processes were being started and stopped 
quickly.  For instance, Xcode runs all its test plans in the debugger when not 
running in a test harness, and runs the tests in parallel.  So it was starting 
& stopping a lot of processes in parallel, and that this should occasionally 
cause the system to stall didn't seem altogether unreasonable.  That was the 
motivation for adding the interrupt timeout in the first place.  That still may 
be true, but it looks like this bug was making the chance of exceeding the 
timeout more likely.

I still think having the timeout is valuable, however, if for no other reason 
than that was how I was able to write a test for the behavior that triggered 
this bug...

(*) BTW, this is not an exact timer.  We don't have a way to stop ReadPacket & 
readjust the wakeup time if the interrupt time happens to be shorter.  And when 
ReadPacket wakes up and finds we have an interrupt pending, but haven't waited 
long enough, formally we should change our timeout to be the amount left to 
wait for the interrupt.  I didn't change this behavior as I don't think we need 
to guarantee that kind of exactness.  As long as we wait at least as long as 
the timeout requested, I think we're doing what folks want, and I don't think 
being more precise is worth the added complexity.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102085

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/TargetProperties.td
  lldb/test/API/functionalities/gdb_remote_client/TestHaltFails.py
  lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
  lldb/unittests/tools/lldb-server/tests/TestClient.cpp

Index: lldb/unittests/tools/lldb-server/tests/TestClient.cpp
===
--- lldb/unittests/tools/lldb-server/tests/TestClient

[Lldb-commits] [PATCH] D102085: Add an "interrupt timeout" to Process, fix a bug in handling interrupt timeouts in

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:370
+  m_should_interrupt(true), m_did_interrupt(false) {
+  SyncWithContinueThread();
+  if (m_acquired)

if "interrupt_timeout" is optional, it can just be passed along to 
SyncWithContinueThread(interrupt_timeout) and avoid the need for 
m_should_interrupt.



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:387
   std::unique_lock lock(m_comm.m_mutex);
-  if (m_comm.m_is_running && !interrupt)
+  if (m_comm.m_is_running && !m_should_interrupt)
 return; // We were asked to avoid interrupting the sender. Lock is not

It we pass take a "llvm::Optional interrupt_timeout = 
llvm::None" as a parameter to this, we can avoid the need for 
m_should_interrupt to exist and can test if it has no value




Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:36
 
-  bool SendAsyncSignal(int signo);
+  bool SendAsyncSignal(int signo, std::chrono::seconds interrupt_timeout);
 

There are a lot of interrupt_timeout parameters in many methods here. Should we 
pass the interrupt timeout to the constructor and keep it as a member variable?



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:55
+   StringExtractorGDBRemote &response,
+   std::chrono::seconds interrupt_timeout);
 

This function and the one above could be made into one function if we change 
the interrupt timeout to be defined as:
```
llvm::Optional interrupt_timeout = llvm::None
```



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:57-60
   PacketResult SendPacketAndReceiveResponseWithOutputSupport(
   llvm::StringRef payload, StringExtractorGDBRemote &response,
-  bool send_async,
+  std::chrono::seconds interrupt_timeout,
   llvm::function_ref output_callback);

Is this function only used when sending async now?



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:73
+// succeed.
+Lock(GDBRemoteClientBase &comm, std::chrono::seconds interrupt_timeout);
 ~Lock();

Combine with above function and change to optional?:
```
llvm::Optional interrupt_timeout = llvm::None
```



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:85
 GDBRemoteClientBase &m_comm;
+std::chrono::seconds m_interrupt_timeout;
 bool m_acquired;

Can't we just set this and update it if the user changes the setting and avoid 
passing "std::chrono::seconds interrupt_timeout" to many of the member 
functions in this class and switch any such parameters to a bool?



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h:325
+  uint32_t length,   // Byte Size of breakpoint or watchpoint
+  std::chrono::seconds interrupt_timeout); // Time to wait for an interrupt
 

We have a ton of new "std::chrono::seconds interrupt_timeout" parameters. Since 
GDBRemoteClientBase already has an ivar for this, can't we just use this and 
pass a bool or no param instead?



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h:514-515
 
-  llvm::Expected SendTraceSupported();
+  llvm::Expected
+  SendTraceSupported(std::chrono::seconds interrupt_timeout);
 

We have a ton of new "std::chrono::seconds interrupt_timeout" parameters. Since 
GDBRemoteClientBase already has an ivar for this, can't we just use this and 
pass a bool or no param instead?



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h:517-518
 
-  llvm::Error SendTraceStart(const llvm::json::Value &request);
+  llvm::Error SendTraceStart(const llvm::json::Value &request,
+ std::chrono::seconds interrupt_timeout);
 

Ditto



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:1213
 llvm::Expected ProcessGDBRemote::TraceSupported() {
-  return m_gdb_comm.SendTraceSupported();
+  return m_gdb_comm.SendTraceSupported(GetInterruptTimeout());
 }

We should just set the interrupt timeout when we create the m_gdb_comm and then 
we won't have to pass this in to each call? We just need to make sure we update 
the timeout if it gets changed during a debug session, but that isn't hard.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:1217
 llvm::Error ProcessGDBRemote::TraceStop(const TraceStopRequest &request) {
-  return m_gdb_comm.SendTraceStop(request);
+  return m_gdb_comm.SendTraceStop(request, GetInterruptTimeout());
 }

ditto and for many calls belo

[Lldb-commits] [PATCH] D102092: [lldb] Enable -Wmisleading-indentation

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: JDevlieghere, aprantl, jasonmolenda.
Herald added a subscriber: mgorny.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Enable `-Wmisleading-indentation` to balance with the LLVM style of optional 
parentheses.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102092

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -158,6 +158,12 @@
 endif ()
 include_directories("${CMAKE_CURRENT_BINARY_DIR}/../clang/include")
 
+# Prevent bugs via useful warnings.
+if (LLVM_ENABLE_WARNINGS)
+  check_cxx_compiler_flag("-Wmisleading-indentation" 
CXX_SUPPORTS_MISLEADING_INDENTATION)
+  append_if(CXX_SUPPORTS_MISLEADING_INDENTATION "-Wmisleading-indentation" 
CMAKE_CXX_FLAGS)
+endif()
+
 # Disable GCC warnings
 check_cxx_compiler_flag("-Wno-deprecated-declarations" 
CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS)
 append_if(CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS 
"-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -158,6 +158,12 @@
 endif ()
 include_directories("${CMAKE_CURRENT_BINARY_DIR}/../clang/include")
 
+# Prevent bugs via useful warnings.
+if (LLVM_ENABLE_WARNINGS)
+  check_cxx_compiler_flag("-Wmisleading-indentation" CXX_SUPPORTS_MISLEADING_INDENTATION)
+  append_if(CXX_SUPPORTS_MISLEADING_INDENTATION "-Wmisleading-indentation" CMAKE_CXX_FLAGS)
+endif()
+
 # Disable GCC warnings
 check_cxx_compiler_flag("-Wno-deprecated-declarations" CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS)
 append_if(CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D102092: [lldb] Enable -Wmisleading-indentation

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

might as well enable it for LLVM more generally? (though probably lldb 
specifically, and lots of llvm more generally, would need cleanup before this 
is enabled? Have you checked if lldb or other parts of llvm build cleanly with 
this warning enabled?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102092

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

[Lldb-commits] [PATCH] D102092: [lldb] Enable -Wmisleading-indentation

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I think some people are already building with this warning + -Werror so this 
should be fine (Apparently that list of people includes me and Stella as I 
fixed a warning that broke the build in https://reviews.llvm.org/D99694#2671667 
)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102092

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

[Lldb-commits] [PATCH] D102085: Add an "interrupt timeout" to Process, fix a bug in handling interrupt timeouts in

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I don't see the change as "adding a bunch of timeouts", since what it mostly 
did was remove a bunch of send_async = false's.  For all the clients that don't 
care about a timeout, I removed the send_async parameter, and for those that 
did I replaced it with a timeout.  There were a few cases at the 
GDBRemoteClientBase layer where routines were calling the SendPacket routines 
with send_async = true w/o requesting that info from ProcessGDBRemote.  To 
those I added a timeout parameter.  That I consider a plus since it made 
explicit which packet request functions were interrupting and which were not.

Given the packet requests that called with send_async = false made up  ~95% of 
all uses, it seemed like marking the few uses that did want to interrupt by the 
fact that they explicitly receive a timeout parameter was cleaner than 
switching everybody over to a Timeout, and then having 95% of the clients still 
have to make up an empty Timeout.

Instead, the rule is now "client functions that are going to interrupt the 
target get passed a timeout, and otherwise you don't have to worry about it".  
That removed a bunch of  boiler-plate and seems clear to me.

We could go further, as you suggest, and add an m_interrupt_timeout to 
GDBRemoteClientBase (*).  That would avoid ever having to pass a timeout into 
the SendPacket client functions in GDBRemoteClientBase.  We already have 
ValueChanged callbacks for Properties.  We'd have to add a virtual 
"InterruptTimeoutChanged" to Process to do the right thing for 
ProcessGDBRemote.  None of that would be hard to do.

In fact, I actually thought about doing that, but I didn't because I like the 
fact that from ProcessGDBRemote, you are able to tell which methods in 
GDBRemoteClientBase interrupt a running process, and which don't by whether 
they take a timeout or not.  If the timeout were in the GDBRemoteClientBase 
class you wouldn't see that.  But that does seems a thing worth knowing.  We 
could preserve that knowledge by adding some naming convention to distinguish 
between interrupting & non-interrupting methods.  But I don't see what that 
really gains us.

OTOH, if the general consensus is that is isn't important to know whether an 
information requesting function in GDBRemoteClientBase is interrupting or not, 
I can certainly add the variable and get it to track the setting changes in 
Process & ProcessGDBRemote.

(*) We would have to add it, the one that's already there belongs to the 
GDBRemoteClientBase::Lock class, which is short-lived, not to the 
GDBRemoteClientBase per se.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h:514-515
 
-  llvm::Expected SendTraceSupported();
+  llvm::Expected
+  SendTraceSupported(std::chrono::seconds interrupt_timeout);
 

clayborg wrote:
> We have a ton of new "std::chrono::seconds interrupt_timeout" parameters. 
> Since GDBRemoteClientBase already has an ivar for this, can't we just use 
> this and pass a bool or no param instead?
Note, m_interrupt_timeout is not an ivar of GDBRemoteClientBase, it's an ivar 
of GDBRemoteClientBase::Lock.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102085

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

[Lldb-commits] [PATCH] D102092: [lldb] Enable -Wmisleading-indentation

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

@dblaikie I have checked lldb, it builds cleanly. In the downstream swift 
branch of lldb there were a couple issues.

I'll check llvm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102092

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

[Lldb-commits] [PATCH] D102085: Add an "interrupt timeout" to Process, fix a bug in handling interrupt timeouts in

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D102085#2745450 , @jingham wrote:

> I don't see the change as "adding a bunch of timeouts", since what it mostly 
> did was remove a bunch of send_async = false's.  For all the clients that 
> don't care about a timeout, I removed the send_async parameter, and for those 
> that did I replaced it with a timeout.  There were a few cases at the 
> GDBRemoteClientBase layer where routines were calling the SendPacket routines 
> with send_async = true w/o requesting that info from ProcessGDBRemote.  To 
> those I added a timeout parameter.  That I consider a plus since it made 
> explicit which packet request functions were interrupting and which were not.
>
> Given the packet requests that called with send_async = false made up  ~95% 
> of all uses, it seemed like marking the few uses that did want to interrupt 
> by the fact that they explicitly receive a timeout parameter was cleaner than 
> switching everybody over to a Timeout, and then having 95% of the clients 
> still have to make up an empty Timeout.
>
> Instead, the rule is now "client functions that are going to interrupt the 
> target get passed a timeout, and otherwise you don't have to worry about it". 
>  That removed a bunch of  boiler-plate and seems clear to me.
>
> We could go further, as you suggest, and add an m_interrupt_timeout to 
> GDBRemoteClientBase (*).  That would avoid ever having to pass a timeout into 
> the SendPacket client functions in GDBRemoteClientBase.  We already have 
> ValueChanged callbacks for Properties.  We'd have to add a virtual 
> "InterruptTimeoutChanged" to Process to do the right thing for 
> ProcessGDBRemote.  None of that would be hard to do.
>
> In fact, I actually thought about doing that, but I didn't because I like the 
> fact that from ProcessGDBRemote, you are able to tell which methods in 
> GDBRemoteClientBase interrupt a running process, and which don't by whether 
> they take a timeout or not.  If the timeout were in the GDBRemoteClientBase 
> class you wouldn't see that.  But that does seems a thing worth knowing.  We 
> could preserve that knowledge by adding some naming convention to distinguish 
> between interrupting & non-interrupting methods.  But I don't see what that 
> really gains us.

I just note when I see a ton of changes to things that can easily be fixed by 
adding an ivar I tend to do it. It is fine that the "bool async" parameter went 
away.

We should probably combine the two functions that only differ by one taking a 
timeout and the other not. They are very similar and can easily use a 
Optional<> on the timeout parameter.

> OTOH, if the general consensus is that is isn't important to know whether an 
> information requesting function in GDBRemoteClientBase is interrupting or 
> not, I can certainly add the variable and get it to track the setting changes 
> in Process & ProcessGDBRemote.
>
> (*) We would have to add it, the one that's already there belongs to the 
> GDBRemoteClientBase::Lock class, which is short-lived, not to the 
> GDBRemoteClientBase per se.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102085

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

[Lldb-commits] [PATCH] D102092: [lldb] Enable -Wmisleading-indentation

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 343773.
kastiglione added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Enable for llvm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102092

Files:
  llvm/cmake/modules/HandleLLVMOptions.cmake


Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -759,6 +759,9 @@
 
   # Enable -Wstring-conversion to catch misuse of string literals.
   add_flag_if_supported("-Wstring-conversion" STRING_CONVERSION_FLAG)
+
+  # Prevent bugs that can happen with llvm's brace style.
+  add_flag_if_supported("-Wmisleading-indentation" MISLEADING_INDENTATION_FLAG)
 endif (LLVM_ENABLE_WARNINGS AND (LLVM_COMPILER_IS_GCC_COMPATIBLE OR CLANG_CL))
 
 if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND NOT LLVM_ENABLE_WARNINGS)


Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -759,6 +759,9 @@
 
   # Enable -Wstring-conversion to catch misuse of string literals.
   add_flag_if_supported("-Wstring-conversion" STRING_CONVERSION_FLAG)
+
+  # Prevent bugs that can happen with llvm's brace style.
+  add_flag_if_supported("-Wmisleading-indentation" MISLEADING_INDENTATION_FLAG)
 endif (LLVM_ENABLE_WARNINGS AND (LLVM_COMPILER_IS_GCC_COMPATIBLE OR CLANG_CL))
 
 if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND NOT LLVM_ENABLE_WARNINGS)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D102092: [lldb] Enable -Wmisleading-indentation

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

Didn't mean to self-accept this, so I resigned as reviewer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102092

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

[Lldb-commits] [PATCH] D102092: [lldb] Enable -Wmisleading-indentation

[David Blaikie via Phabricator via lldb-commits]

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

Sounds good to me if - if the build (of ~everything in the monorepo) is clean - 
and do please keep an eye on the buildbots when this is committed as there's 
likely to be cleanup here and there still required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102092

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

[Lldb-commits] [PATCH] D102085: Add an "interrupt timeout" to Process, fix a bug in handling interrupt timeouts in

[Jim Ingham via Phabricator via lldb-commits]

jingham updated this revision to Diff 343785.
jingham added a comment.

I coalesced the timeout & no-timeout versions of 
GDBRemoteClientBase::SendPacketAndWaitForResponse and 
GDBRemoteClientBase::Lock::Lock as you suggested.  I didn't use an Optional.  
That seems overkill when there's a perfectly good sentinel value for "don't 
interrupt": timeout of 0 seconds.  So I just made the timeout in these two 
cases have a default value of 0 seconds, and used that to mean no interrupt.  
That way passing no timeout means "don't interrupt" and passing a non-zero one 
means interrupt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102085

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/TargetProperties.td
  lldb/test/API/functionalities/gdb_remote_client/TestHaltFails.py
  lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
  lldb/unittests/tools/lldb-server/tests/TestClient.cpp

Index: lldb/unittests/tools/lldb-server/tests/TestClient.cpp
===
--- lldb/unittests/tools/lldb-server/tests/TestClient.cpp
+++ lldb/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -193,7 +193,7 @@
   PacketResult expected_result) {
   StringExtractorGDBRemote response;
   GTEST_LOG_(INFO) << "Send Packet: " << message.str();
-  PacketResult result = SendPacketAndWaitForResponse(message, response, false);
+  PacketResult result = SendPacketAndWaitForResponse(message, response);
   response.GetEscapedBinaryData(response_string);
   GTEST_LOG_(INFO) << "Read Packet: " << response_string;
   if (result != expected_result)
Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -385,8 +385,9 @@
   TraceSupportedResponse trace_type;
   std::string error_message;
   auto callback = [&] {
+std::chrono::seconds timeout(10);
 if (llvm::Expected trace_type_or_err =
-client.SendTraceSupported()) {
+client.SendTraceSupported(timeout)) {
   trace_type = *trace_type_or_err;
   error_message = "";
   return true;
Index: lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
@@ -55,6 +55,8 @@
   }
 
 protected:
+  // We don't have a process to get the interrupt timeout from, so make one up.
+  static std::chrono::seconds g_timeout;
   TestClient client;
   MockServer server;
   MockDelegate delegate;
@@ -72,6 +74,8 @@
   }
 };
 
+std::chrono::seconds GDBRemoteClientBaseTest::g_timeout(10);
+
 } // end anonymous namespace
 
 TEST_F(GDBRemoteClientBaseTest, SendContinueAndWait) {
@@ -103,7 +107,7 @@
   StringExtractorGDBRemote continue_response, response;
 
   // SendAsyncSignal should do nothing when we are not running.
-  ASSERT_FALSE(client.SendAsyncSignal(0x47));
+  ASSERT_FALSE(client.SendAsyncSignal(0x47, g_timeout));
 
   // Continue. After the run packet is sent, send an async signal.
   std::future continue_state = std::async(
@@ -112,8 +116,9 @@
   ASSERT_EQ("c", response.GetStringRef());
   WaitForRunEvent();
 
-  std::future async_result = std::async(
-  std::launch::async, [&] { return client.SendAsyncSignal(0x47); });
+  std::future async_result = std::async(std::launch::async, [&] {
+return client.SendAsyncSignal(0x47, g_timeout);
+  });
 
   // First we'll get interrupted.
   ASSERT_EQ(PacketResult::Success, server.GetPacket(response));
@@ -133,7 +138,6 @@
 
 TEST_F(GDBRemoteClientBaseTest, SendContinueAndAsyncPacket) {
   StringExtractorGDBRemote continue_response, async_response, response;
-  const bool send_async = true;
 
   // Continue. After the run packet is sent, send an async packet.
   std::future continue_state = std::async(
@@ -143,13 +147,12 @@
   WaitForRunEvent();
 
   // Sending without async enabled should fail.
-  ASSERT_EQ(
-  PacketResult::ErrorSendFailed,
-  client.SendPacketAndWaitForResponse("qTest1", response, !send_async));
+  ASSERT_EQ(Pa

[Lldb-commits] [PATCH] D102085: Add an "interrupt timeout" to Process, fix a bug in handling interrupt timeouts in

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

BTW, I generally agree that passing values in parameters that are always going 
to be used internally anyway is ugly and it's better to store them in an ivar.  
But in this case, the value was going to get used in some cases and not in 
others, which storing it as a parameter would have hidden.  That's what I was 
trying to avoid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102085

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