[PATCH] D157479: [Clang][DebugInfo] Emit narrower base types for structured binding declarations that bind to struct bitfields

aprantl added a comment. New test LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157479/new/ https://reviews.llvm.org/D157479 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D157479: [Clang][DebugInfo] Emit narrower base types for structured binding declarations that bind to struct bitfields

aprantl added a comment. > If no suitable integer type is found in the target, no debug information is > emitted anymore in order to prevent wrong debug output. Why is emitting a bitfield type not an option? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D157479: [Clang][DebugInfo] Emit narrower base types for structured binding declarations that bind to struct bitfields

aprantl added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp:525 +// CHECK: !202 = !DILocation(line: 279, column: 8, scope: !194) +// CHECK: !203 = !DILocation(line: 279, column: 17, scope: !194) +// CHECK: !204 = !DILocation(line: 2

[PATCH] D153362: [clang][DebugInfo] Emit DW_AT_defaulted for defaulted C++ member functions

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Not sure if this is the driving motivation, but this would probably allow LLDB's expression evaluator to synthesize a default implementation for these functions if they were optimized away.

[PATCH] D153282: [clang][DebugInfo] Emit DW_AT_deleted on any deleted member function

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Since a boolean flag is effectively free in DWARF as it can be stored in the abbreviations, this looks like a good change to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D152708: [RFC][Draft] Enable primitive support for Two-Level Line Tables in LLVM

aprantl added a comment. Thanks for prototyping this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152708/new/ https://reviews.llvm.org/D152708 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

aprantl added a comment. If we can come up with a counterexample where the heuristic in this patch is doing the wrong thin then I think emitting a DW_AT_LLVM_no_unique_address attribute sounds reasonable to me. But otherwise I don't see a problem with using a heuristic. Repository: rG LLVM

[PATCH] D146595: [clang] Add "debug_trampoline" attribute

aprantl added a comment. In D146595#4278361 , @dblaikie wrote: > In D146595#4235340 , @augusto2112 > wrote: > >> In D146595#4235333 , @aprantl >> wrote: >> >>> I hope I'

[PATCH] D146595: [clang] Add "transparent_stepping" attribute

aprantl added a comment. > Is there some place (bug, discourse thread, etc) where the broader direction > is discussed? I want to checkin on the design decisions/alternatives without > fragmenting this across multiple reviews/losing context/etc? No, I believe that all the relevant discussion ha

[PATCH] D146595: [clang] Add "transparent_stepping" attribute

aprantl added a comment. I think `debug_trampoline` both captures the semantics and makes it clear that this is related to debugging. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146595/new/ https://reviews.llvm.org/D146595 _

[PATCH] D146595: [clang] Add "transparent_stepping" attribute

aprantl added a comment. I hope I'm not kicking off a long bike-shedding thread, but I would propose to either call the attribute `transparent` to match the DWARF attribute name, or if we want to be more descriptive, `debug_transparent`, or `transparentdebug` to fit better with other attributes

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

aprantl added a subscriber: rnk. aprantl added a comment. In D146595#4225630 , @aaron.ballman wrote: > It's less about other debug formats and more about user experience. My two > big concerns are: 1) writing multiple attributes to do the same thing is

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

aprantl added a comment. > We don't want to have one attribute per debug format, because that's not > going to scale. LLVM supports outputting a total of 2 debug info formats. > So how do we validate that this attribute should be exposed to users *now* > before we know what the story is for ot

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

aprantl added a comment. > Why should this feature be limited to DWARF? Don't we have the same kind of > stepping behavior issue with PDB files, for example? That's not what I was trying to say. Yes, `DW_AT_trampoline` is a DWARF feature. But the "target function" LLVM IR feature is not necessa

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

aprantl added a comment. > I guess it was implemented there first/ported to clang with the fortran effort Yeah my understanding is that the LLVM feature was added for flang, and so I'm not sure what the targeted debugger is, I believe there are some non-GDB/LLDB debuggers targeting the HPC mark

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

aprantl added a comment. In D146595#4225048 , @dblaikie wrote: > I know this is a bit of a redirection/scope creep/etc - but I'd quite like to > see a solution that is likely to be usable for the "std::function" problem > (stepping into std::function sh

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

aprantl added a comment. > If it's the latter, then, yeah, some "transparent to debugger" source > attribute might be appropriate - that lowers to a bit on the DISubprogram and > instructs LLVM to, if the function definition ends up lowering to a > trampoline, mark that for the debugger so it's

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

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. So this attribute will lower into a `DW_AT_trampoline("target_func_name")` attribute on the `DW_TAG_subprogram` of the function definition? The debug info parts LGTM. It would be nice to hea

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

aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2640 if (!D || !D->isCompleteDefinition()) -return FwdDecl; +return {FwdDecl, nullptr}; I'm curious if this works if we encounter a forward declaration, early exit here, th

[PATCH] D143984: [DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI (1/7)

aprantl added inline comments. Comment at: llvm/include/llvm/IR/DIBuilder.h:76 -/// Each subprogram's preserved labels. -DenseMap> PreservedLabels; +SmallVectorImpl & +getSubprogramNodesTrackingVector(const DIScope *S) { Do you need a writeable

[PATCH] D145077: [clang][DebugInfo] Support DW_AT_LLVM_preferred_name attribute

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. I originally was hoping we wouldn't have to introduce a new attribute for this, but it looks like there are legitimate concerns about the alternatives, so in that sense this looks good! Re

[PATCH] D144181: [clang][DebugInfo] Add abi-tags on constructors/destructors as LLVM annotations

aprantl added a comment. Size-wise this looks like an acceptable increase. If we created a new DW_AT_LLVM_abi_tag, we could save an extra 4 bytes (assuming DW_FORM_strp) per DIE. That might be worth it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D143501: [clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

aprantl added a comment. > Alternatively, I suppose, the DWARF emission could just look at the preferred > name and use that as the DW_AT_type in all cases anyway? Avoids needing a new > attribute, etc, though would be a bit quirky in its own way. Am I understanding you correctly that you sugge

[PATCH] D143921: [debug-info][codegen] Prevent creation of self-referential SP node

aprantl accepted this revision. aprantl added a comment. Test looks good! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143921/new/ https://reviews.llvm.org/D143921 ___ cfe-commits mailing list cfe-commi

[PATCH] D143921: [debug-info][codegen] Prevent creation of self-referential SP node

aprantl added a comment. In D143921#4123218 , @fdeazeve wrote: > Any testing suggestions here? I can use what we have on GH (cpp -> codegen > test), but I'm not sure if there's a finer grained test we could use. I was thinking of a very small IR test si

[PATCH] D143921: [debug-info][codegen] Prevent creation of self-referential SP node

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: llvm/lib/IR/Verifier.cpp:1404 +CheckDI(!N.getRawDeclaration(), +"subprogram declaration must not have a declaration field"); } --

[PATCH] D143501: [WIP][clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

aprantl added a comment. Nice! Does `expr -- std::basic_string s` still work after this change? Not that anyone would want to type this over `std::string` ... Comment at: clang/test/CodeGen/debug-info-preferred-names.cpp:1 +// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limite

[PATCH] D142632: [clang][TypePrinter] Support expression template arguments when checking defaultedness

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/AST/TypePrinter.cpp:2018 + + // Can't evaluate value-dependent expressions so bail early + Expr const *pattern_expr = Pattern.getAsExpr(); -

[PATCH] D142243: [CodeGen] bugfix: ApplyDebugLocation goes out of scope before intended

aprantl added a comment. Code LGTM, it would be nice if we could further reduce the test case. Comment at: clang/test/CodeGenObjC/objc-arc-ubsan-debugging.m:2 +// RUN: %clang -x objective-c -target arm64-apple-macos12.0 -fobjc-arc -std=gnu99 -O0 -fsanitize=undefined -fsanitiz

[PATCH] D140195: [Clang][CGDebugInfo][ObjC] Mark objc bitfields with the DIFlagBitfield flag

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. > Unluckly, I wasn't able to test the combination of > https://reviews.llvm.org/D96334 and this patch together. If you have some > pointers about how to trigger a test job on "Green Dragon",

[PATCH] D137838: [Support] Move TargetParsers to new component

aprantl added a comment. Sorry, I pasted in the wrong hash: 6bdf378dcd349d97152846bb687c1d1de511d138 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/ https://reviews.ll

[PATCH] D137838: [Support] Move TargetParsers to new component

aprantl added a comment. I think I fixed it in a685bb8e333e , but please take another look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/ https://reviews.llvm.org/D1

[PATCH] D137838: [Support] Move TargetParsers to new component

aprantl added a comment. Can you please update `llvm/include/llvm/module.modulemap` for this change or revert the patch? This is breaking all bots that build with `-DLLVM_ENABLE_MODULES=On`. For example: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/lastFailedBuild/consoleFull#1110

[PATCH] D140195: [Clang][CGDebugInfo][ObjC] Mark objc bitfields with the DIFlagBitfield flag

aprantl added a comment. How does this affect the generated DWARF? Have you tested that this does the right thing in LLDB? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140195/new/ https://reviews.llvm.org/D140195

[PATCH] D139986: [clang][TypePrinter] Teach isSubstitutedDefaultArgument about integral types

aprantl added inline comments. Comment at: clang/test/CodeGenObjCXX/encode.mm:93-94 // FIXME: This difference is due to D76801. It was probably an unintentional change. Maybe we want to undo it? - // CHECKCXX98: @_ZN11rdar93574002ggE ={{.*}} constant [49 x i8] c"{vector >=[

[PATCH] D139986: [clang][TypePrinter] Teach isSubstitutedDefaultArgument about integral types

aprantl accepted this revision. aprantl added inline comments. Comment at: clang/test/CodeGenObjCXX/encode.mm:93-94 // FIXME: This difference is due to D76801. It was probably an unintentional change. Maybe we want to undo it? - // CHECKCXX98: @_ZN11rdar93574002ggE ={{.*}} c

[PATCH] D139986: [clang][TypePrinter] Teach isSubstitutedDefaultArgument about integral types

aprantl added inline comments. Comment at: clang/test/CodeGenObjCXX/encode.mm:93-94 // FIXME: This difference is due to D76801. It was probably an unintentional change. Maybe we want to undo it? - // CHECKCXX98: @_ZN11rdar93574002ggE ={{.*}} constant [49 x i8] c"{vector >=[

[PATCH] D139985: [clang][AST][NFC] Move isSubstitutedDefaultArgument out of TypePrinter into separate component

aprantl added inline comments. Comment at: clang/include/clang/AST/TemplateUtils.h:19 +namespace clang { +namespace TemplateUtils { +/// Make a best-effort determination of whether the type T can be produced by Michael137 wrote: > dblaikie wrote: > > aprantl wrot

[PATCH] D139986: [clang][TypePrinter] Teach isSubstitutedDefaultArgument about integral types

aprantl added inline comments. Comment at: clang/test/CodeGenObjCXX/encode.mm:93-94 // FIXME: This difference is due to D76801. It was probably an unintentional change. Maybe we want to undo it? - // CHECKCXX98: @_ZN11rdar93574002ggE ={{.*}} constant [49 x i8] c"{vector >=[

[PATCH] D139953: [llvm][DebugInfo] Backport DW_AT_default_value for template args

aprantl added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-template-parameter.cpp:7 // RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=4 -triple x86_64 %s -O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s --check-prefixes=CHECK,CXX

[PATCH] D139953: [llvm][DebugInfo] Backport DW_AT_default_value for template args

aprantl added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-template-parameter.cpp:7 // RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=4 -triple x86_64 %s -O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s --check-prefixes=CHECK,CXX

[PATCH] D139985: [clang][AST][NFC] Move isSubstitutedDefaultArgument out of TypePrinter into separate component

aprantl added a comment. Seems fine to me, I just have one inline comment about the name of the namespace. Comment at: clang/include/clang/AST/TemplateUtils.h:19 +namespace clang { +namespace TemplateUtils { +/// Make a best-effort determination of whether the type T can be pr

[PATCH] D139988: [clang][DebugInfo] Re-use clang::TemplateUtils to determine guide DW_AT_default_value for template parameters

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. That looks nicer than the old code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139988/new/ https://reviews.llvm.org/D139988 __

[PATCH] D138597: DebugInfo: Add/support new DW_LANG codes for recent C and C++ versions

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This minimal patch LGTM. Comment at: llvm/lib/IR/DIBuilder.cpp:159 - assert(((Lang <= dwarf::DW_LANG_Fortran08 && Lang >= dwarf::DW_LANG_C89) || + assert(((Lang <= dwar

[PATCH] D138597: DebugInfo: Add/support new DW_LANG codes for recent C and C++ versions

aprantl added a comment. Agreeing with @probinson Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138597/new/ https://reviews.llvm.org/D138597 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D138597: DebugInfo: Add/support new DW_LANG codes for recent C and C++ versions

aprantl added a comment. > @aprantl do you have an opinion on this? I tend to lean to the pedantic side > on this kind of thing, but I'm persuadable. As long as LLDB can deal with it I'm fine either way. Emitting the separated DWARF 6 attribute as an extension sounds fine to me. =

[PATCH] D136041: [clang][DebugInfo] Emit DISubprogram for extern functions with reserved names

aprantl accepted this revision as: aprantl. aprantl added a comment. This revision is now accepted and ready to land. I think this is reasonable. Ideally the LLVM and Clang patches should be two independent commits. Please wait a few days for others to chime in though. Do you have any idea how m

[PATCH] D133158: [NFC] Make MultiplexExternalSemaSource own sources

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This looks plausible to me — did you build clang-tools-extra & lldb to make sure nothing else needs to be updated? Comment at: clang/include/clang/Sema/MultiplexExternalSe

[PATCH] D133158: [NFC] Make MultiplexExternalSemaSource own sources

aprantl added inline comments. Comment at: clang/include/clang/Sema/MultiplexExternalSemaSource.h:53 /// - MultiplexExternalSemaSource(ExternalSemaSource& s1, ExternalSemaSource& s2); + MultiplexExternalSemaSource(ExternalSemaSource* S1, ExternalSemaSource* S2); --

[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

aprantl added a comment. I think we can "fix" the test with the following patch: diff --git a/lldb/test/API/functionalities/unused-inlined-parameters/main.c b/lldb/test/API/functionalities/unused-inlined-parameters/main.c index f2ef5dcc213d..9b9f95f6c946 100644 --- a/lldb/test/API/function

[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

aprantl added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.h:1 +//===-- Coroutines.h ---*- C++//-*-===// +// typo: `-*- C++ -*-` Comment at: lldb/source/Plugins/Language

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

aprantl added a comment. @cjdb Would you mind reverting the patch until we figured out a solution to unblock the CI? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116203/new/ https://reviews.llvm.org/D116203 __

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

aprantl added a subscriber: Michael137. aprantl added a comment. Sorry for the silence — I was out on vacation. @Michael137 has recently started looking into improving support for C++ lambdas in LLDB. Michael, would you be interested in taking a fresh look at this and figure out what the requir

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

aprantl added a comment. In D123319#3517966 , @dblaikie wrote: > In D123319#3506745 , @shafik wrote: > >> > > What does the linkage name do for your use case? Which cases are missing > linkage names/where do the

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

aprantl accepted this revision. aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:403 case DW_AT_type: - type = form_value; + if (!type.IsValid()) +type = form_value; Could you add a com

[PATCH] D126689: [IR] Enable opaque pointers by default

aprantl added a comment. I was able to update LLDB https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/44252/. We can leave this in. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126689/new/ https://reviews.llvm.org/D126689 ___

[PATCH] D126689: [IR] Enable opaque pointers by default

aprantl added a comment. It looks like this patch has broken 168 tests in LLDB: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/44239/changes#41d5033eb162cb92b684855166cabfa3983b74c6 I'm going to dig a little deeper, but I might ask you to revert this until we can figure out a solution

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

aprantl added a comment. I'm going to revert the patch now. This is not just breaking LLDb, but also clang itself on Darwin platforms. I think we need to be more careful to separate out the enabling of Clang C++ modules and C++20 modules. Either by having `-fmodules-ts` control the `HaveModules

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

aprantl added a comment. In any case, I would appreciate it if we could revert this patch until we found a solution! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120540/new/ https://reviews.llvm.org/D120540 __

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

aprantl added a comment. Should we introduce a `-fno-modules-ts` option on top? Any better suggestions? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120540/new/ https://reviews.llvm.org/D120540 ___ cfe-

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

aprantl added a comment. I think the problem might be that previously on Darwin `-fcxx-modules` was used to turn on C++ Clang modules (which was not implied by `-fmodules`) without turning of C++20 (ts) modules, and after this patch `-fcxx-modules` implies `-fmodules-ts`. Does that sound plausi

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

aprantl added a comment. https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/44167/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120540/new/ https://reviews.llvm.org/D120540 ___ cfe-commits mailin

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

aprantl added a comment. This patch broke a whole bunch tests in the LLDB testsuite. I'm trying to figure out what exactly the semantics of the change are, particularly on Darwin, where `-fmodules` doesn't imply `-fcxx-modules`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D125839: [gmodules] Skip CXXDeductionGuideDecls when visiting FunctionDecls in DebugTypeVisitor

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Adding the debug-info group. I don't really know enough about deduction guides to comment on the implications, but if it fixes the crash it seems to be a strict improvement. Maybe wait anoth

[PATCH] D121100: [clang][DebugInfo] clang should not generate DW_TAG_subprogram entry without DW_AT_name

aprantl added a comment. In D121100#3530683 , @alok wrote: > GNU gdb is now modified to accept functions with linkage name. > > commit 6f9b09edaee43ea34d34b1998fe7b844834f251a > Author: Alok Kumar Sharma > Date: Sun May 22 21:46:06 2022 +0530 Ni

[PATCH] D125695: [clang][DebugInfo] Allow function-local type-like entities to be scoped within a lexical block (5/5)

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Sgtm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125695/new/ https://reviews.llvm.org/D125695

[PATCH] D125694: [clang][DebugInfo] Allow function-local statics to be scoped within a lexical block (4/5)

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This looks very good, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125694/new/ https://reviews.llvm.org/D125694

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

aprantl added a comment. I think this mostly looks good. I left comment about the test inline. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123534/new/ https://reviews.llvm.org/D123534 ___ cfe-commits m

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.h:536 + /// Create and attach debuginfo to a the provided string literal GV. + void AddStringLiteralDebugInfo(llvm::GlobalVariable *GV, StringRef Name, I would appreciate a comment expla

[PATCH] D123787: [clang][OpenMP][DebugInfo] Debug support for TLS variables when present in OpenMP consructs

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Oh, it's a //global// variable that supposed to shadow the function parameter! Thanks for clarifying. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123787/new/ https://reviews.llvm

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. The new test looks good (I would replace the CHECK-NEXT with CHECK though). It sounds like there was no objections for doing this for lambdas. CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D123787: [clang][OpenMP][DebugInfo] Debug support for TLS variables when present in OpenMP consructs

aprantl added a comment. > For an example, if thread local variable is present in copyin clause > (testcase attached with the patch), parameter with same name is generated as parameter to artificial function. When user inquires the thread Local variable, its debug info is hidden by the paramete

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1684 + +if (AT->isDeduced() && ThisPtr->getPointeeCXXRecordDecl()->isLambda()) + Elts.push_back(getOrCreateType(AT->getDeducedType(),Unit)); aprantl wrote: > Can you add a comme

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

aprantl added a comment. > Are there cases where we emit debug info and we don't have the deduced type? I don't think that would happen for a lambda. https://dwarfstd.org/ShowIssue.php?issue=131217.1 specifically calls out method declarations without definitions, which makes sense to me. CHANG

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

aprantl added a comment. I think this is reasonable, but could you add a testcase? Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1684 + +if (AT->isDeduced() && ThisPtr->getPointeeCXXRecordDecl()->isLambda()) + Elts.push_back(getOrCreateType(AT->getDeducedType(),Unit));

[PATCH] D120989: Support debug info for alias variable

aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3922 + return cast(GVE); +return dyn_cast_or_null(N); + } if we don't expect anything but non-null DINodes to be in the cache, then this whole condition should be ``` return c

[PATCH] D121100: [clang][DebugInfo] clang should not generate DW_TAG_subprogram entry without DW_AT_name

aprantl added a comment. Ideally, this would be fixed in GDB. If that's really not feasibly I would rather like to see this as a gdb debugger tuning instead f the default behavior. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121100/new/ https://

[PATCH] D120989: Support debug info for alias variable

aprantl added a comment. This looks mostly fine to me, I have a couple of superficial comments inline. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3914 + // imported declaration + auto IE = ImportedDeclCache.find(D->getCanonicalDecl()); Nit: The LLVM codi

[PATCH] D113718: Don't append the working directory to absolute paths

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2cd9a86da54f: Don't append the working directory to absolute paths (authored by aprantl). Herald added a project: clang. Repository: rG LLVM Githu

[PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4647 +const bool UsePointerValue) { + assert(CGM.getCodeGenOpts().hasRedu

[PATCH] D119850: Darwin: introduce a global override for debug prefix map entries

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0604d86c07ab: Darwin: introduce a global override for debug prefix map entries. (authored by aprantl). Herald added a project: clang. Changed prior

[PATCH] D119850: Darwin: introduce a global override for debug prefix map entries

aprantl added a comment. In D119850#3326266 , @thakis wrote: > (I wish there was a flag that said "clang, never ever read any env vars > please". Some people think that builds should be deterministic and not depend > on random env vars people might have

[PATCH] D119850: Darwin: introduce a global override for debug prefix map entries

aprantl added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:686 +return; + if (!StringRef(GlobalRemapEntry).contains('=')) +D.Diag(diag::err_drv_invalid_argument_to_option) arphaman wrote: > Suggestion: It might be good to factor out

[PATCH] D119850: Darwin: introduce a global override for debug prefix map entries

aprantl updated this revision to Diff 408944. aprantl added a comment. Refactored code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119850/new/ https://reviews.llvm.org/D119850 Files: clang/include/clang/Driver/ToolChain.h clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Driver/

[PATCH] D119850: Darwin: introduce a global override for debug prefix map entries

aprantl created this revision. aprantl added reviewers: dexonsmith, arphaman. aprantl requested review of this revision. This patch adds a new Darwin clang driver environment variable in the spirit of RC_DEBUG_OPTIONS, called RC_DEBUG_PREFIX_MAP, which allows a meta build tool to add one additio

[PATCH] D118070: Make lld-link work in a non-MSVC shell

aprantl added a comment. Unfortunately this seems to break the modular build. Would you mind taking a look? I'm going to revert the patch to get the bots going again. https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/41306/console Comment at: llvm/include/llvm/Support

[PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4647 +const bool UsePointerValue) { + assert(CGM.getCodeGenOpts().hasReducedDebugInfo()); + assert(!LexicalBlockStack.empty() && "Region stack mismatch,

[PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

aprantl added a comment. I think this looks pretty good! I have a few questions inside. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4667 + + SmallVector Expr; + AppendAddressSpaceXDeref(AddressSpace, Expr); 13 seems to be unnecessarily high. Shouldn't 1 be

[PATCH] D116790: C++ -gmodules .pcm files don't have the same DW_AT_language dialect

This revision was automatically updated to reflect the committed changes. Closed by commit rGeb200e584ece: Emit the C++ dialect in -gmodules .pcm files. (authored by aprantl). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D116113: Add LLDB synthetic child and summary scripts for llvm::SmallVector, llvm::Optional, llvm::ErrorOr and llvm::Expected.

aprantl added a comment. How does this relate to the code in `llvm/utils/lldbDataFormatters.py` and should it be added there instead? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116113/new/ https://reviews.llvm.org/D116113 _

[PATCH] D113743: [RFC][clang][DebugInfo] Allow function-local statics and types to be scoped within a lexical block

aprantl added a comment. This looks reasonable to me. Thanks! Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:234 + if (!LexicalBlockStack.empty()) +LexicalBlockMap[&D] = LexicalBlockStack.back(); +} maybe use `LexicalBlockMap.insert({&D, LexicalBlockStack.b

[PATCH] D113718: Don't append the working directory to absolute paths

aprantl created this revision. aprantl added reviewers: dblaikie, rastogishubham, kastiglione. aprantl added a project: debug-info. aprantl requested review of this revision. This fixes a bug that happens when using -fdebug-prefix-map to remap an absolute path to a relative path. Since the path w

[PATCH] D106585: Fix clang debug info irgen of i128 enums

aprantl added a comment. In D106585#3110131 , @glandium wrote: > This broke determinism when building Firefox. I'm curious: can you share an example dwarfdump diff that shows what is non-deterministic? Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D111477: DO NOT SUBMIT: workaround for context-sensitive use of non-type-template-parameter integer suffixes

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. I think this is fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111477/new/ https://reviews.llvm.org/D111477

[PATCH] D109541: Increase expected line number for ExtDebugInfo.cpp

aprantl added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109541/new/ https://reviews.llvm.org/D109541 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D109940: Apply proper source location to fallthrough switch cases

This revision was automatically updated to reflect the committed changes. Closed by commit rG843390c58ae6: Apply proper source location to fallthrough switch cases. (authored by aprantl). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://rev

[PATCH] D107024: [DIBuilder] Do not replace empty enum types

aprantl accepted this revision. aprantl 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/D107024/new/ https://reviews.llvm.org/D107024 __

[PATCH] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

aprantl added a comment. @teemperor is there a reason why the fixed patch never landed? This looks a good improvement to have. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80878/new/ https://reviews.llvm.org/D80878 ___ cfe-commits mailing l

[PATCH] D107024: [DIBuilder] Do not replace empty enum types

aprantl added a comment. I think that looks fine — I wonder if we should change the IR pretty printer to display empty arrays inline as `elements: !{}`, too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107024/new/ https://reviews.llvm.org/D10702

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2693 +other_die = dcu->LookupAddress( +symtab->SymbolAtIndex(symbol_indexes[index])->GetFileAddress()); + } should there be an early exit

  1   2   3   4   5   6   >