[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

[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

[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:

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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,

[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!

[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

[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

[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

[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

[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

[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

[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

[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/

[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/

[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:

[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 ={{.*}}

[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

[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

[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

[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

[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 <=

[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

[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

[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:

[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 ---

[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:

[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

[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

[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

[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

[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

[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 ___

[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

[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

[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

[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

[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

[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

[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/

[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

[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

[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

[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.

[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()) +

[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

[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/

[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

[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: Dont append the working directory to absolute paths (authored by aprantl). Herald added a project: clang. Repository: rG LLVM

[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) { +

[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

[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

[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

[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

[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:

[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

[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[] = LexicalBlockStack.back(); +} maybe use `LexicalBlockMap.insert({,

[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

[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

[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

[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:

[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

[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/

[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   >