[PATCH] D120989: Support debug info for alias variable

2022-03-10 Thread Adrian Prantl via Phabricator via cfe-commits
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] D121100: [clang][DebugInfo] clang should not generate DW_TAG_subprogram entry without DW_AT_name

2022-03-10 Thread Adrian Prantl via Phabricator via cfe-commits
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

2022-03-18 Thread Adrian Prantl via Phabricator via cfe-commits
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] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

2022-04-07 Thread Adrian Prantl via Phabricator via cfe-commits
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] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

2022-04-07 Thread Adrian Prantl via Phabricator via cfe-commits
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

2022-04-14 Thread Adrian Prantl via Phabricator via cfe-commits
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] D123787: [clang][OpenMP][DebugInfo] Debug support for TLS variables when present in OpenMP consructs

2022-04-14 Thread Adrian Prantl via Phabricator via cfe-commits
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

2022-04-21 Thread Adrian Prantl via Phabricator via cfe-commits
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] D119178: Add support for generating debug-info for structured bindings of structs and arrays

2022-02-08 Thread Adrian Prantl via Phabricator via cfe-commits
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] D119178: Add support for generating debug-info for structured bindings of structs and arrays

2022-02-08 Thread Adrian Prantl via Phabricator via cfe-commits
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] D118070: Make lld-link work in a non-MSVC shell

2022-02-11 Thread Adrian Prantl via Phabricator via cfe-commits
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] D119850: Darwin: introduce a global override for debug prefix map entries

2022-02-15 Thread Adrian Prantl via Phabricator via cfe-commits
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] D119850: Darwin: introduce a global override for debug prefix map entries

2022-02-15 Thread Adrian Prantl via Phabricator via cfe-commits
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

2022-02-15 Thread Adrian Prantl via Phabricator via cfe-commits
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

2022-02-16 Thread Adrian Prantl via Phabricator via cfe-commits
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

2022-02-16 Thread Adrian Prantl via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0604d86c07ab: Darwin: introduce a global override for debug prefix map entries. (authored by aprantl). Herald added a project: clang. Changed prior

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

2022-02-16 Thread Adrian Prantl via Phabricator via cfe-commits
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] D123787: [clang][OpenMP][DebugInfo] Debug support for TLS variables when present in OpenMP consructs

2022-04-22 Thread Adrian Prantl via Phabricator via cfe-commits
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] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

2022-05-13 Thread Adrian Prantl via Phabricator via cfe-commits
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] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

2022-05-13 Thread Adrian Prantl via Phabricator via cfe-commits
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] D66667: Debug Info: Support for DW_AT_export_symbols for anonymous structs

2019-08-23 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments. Comment at: test/CodeGenCXX/debug-info-export_symbols.cpp:3 + +// CHECK-DAG: !DICompositeType({{.*}}flags: DIFlagExportSymbols | DIFlagTypePassByValue +struct A { This should just be `CHECK:` if it stands by itself. It would be be

[PATCH] D66667: Debug Info: Support for DW_AT_export_symbols for anonymous structs

2019-08-26 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: test/CodeGenCXX/debug-info-export_symbols.cpp:4 +// CHECK: [[SCOPE:![0-9]+]] = distinct !DICompositeType({{.*}}flags: DIFlagTypePassByValue +// CHECK: !DIC

[PATCH] D66121: Debug Info: Nest Objective-C property function decls inside their container.

2019-08-29 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl updated this revision to Diff 218000. aprantl added a comment. Herald added a subscriber: arphaman. Here is a work-in-progress alternative patch that attacks the problem by changing the AST generation to have an ObjCMethodDecl for the property accessors inside the implementation as sugge

[PATCH] D46791: Make -gsplit-dwarf generally available

2019-09-03 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments. Comment at: test/Driver/split-debug.c:28 +// MACOSX-CHECK-ACTIONS: objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo" +// MACOSX-CHECK-ACTIONS: objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o" split dwarf does not make sense on macOS

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-09-05 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. Nice! Are you planning to address the FIXME's in a later update of this patch? Comment at: clang/lib/Serialization/ASTWriter.cpp:1767 bool IsTopLevelModuleMap; + uint32_t ContentHash[2]; }; Why is this not a uint64_t? ===

[PATCH] D67373: Don't emit .gnu_pubnames when tuning for LLDB

2019-09-09 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl created this revision. aprantl added reviewers: jasonmolenda, dblaikie, friss, emaste, JDevlieghere. Herald added a project: clang. LLDB reads the various `.apple*` accelerator tables which should make `.gnu_pubnames` redundant. This changes the Clang driver to no longer pass `-ggnu-pubn

[PATCH] D67373: Don't emit .gnu_pubnames when tuning for LLDB

2019-09-09 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. In D67373#1663924 , @dblaikie wrote: > Have you got a link to the original thread where this was discussed/I > mentioned it? Want to page in some context to double-check if I had any ideas > that might've let this simplify things

[PATCH] D35923: Fix PR32332 - PCM nondeterminism with builtins caused by global module index

2017-07-28 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. After thinking about this some more, I think it should be safe to use the global module index as a negative cache like in this example. The conditions under which a module is rebuilt should not affect the contents, and if the module were out-of-date it would have been r

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

2017-08-22 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments. Comment at: lib/CodeGen/CGVTables.cpp:157 + if (DebugInfo) +DebugInfo->replaceTemporaryNodes(); + Have you looked at what it would take to only finalize the nodes reachable via the function? Otherwise — have you audited that t

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

2017-08-22 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. Performance-wise the change is fine because it does the same amount of work, but I would prefer someone to audit the code to make sure that we aren't uniquing a node while we still want to make changes to it. Unfortunately testcases for these issues will involve impossi

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

2020-01-09 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4516 void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue &Init) { - assert(DebugKind >= codegenoptions::LimitedDebugInfo); + assert(CGM.getCodeGenOpts().isFullDebug()); if (VD-

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

2020-01-10 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4516 void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue &Init) { - assert(DebugKind >= codegenoptions::LimitedDebugInfo); + assert(CGM.getCodeGenOpts().isFullDebug()); if (VD-

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

2020-01-13 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. In D71451#1816269 , @Jac1494 wrote: > ping I'd be curious to the answer to David's questions. If the size increase is because of unused extern variables coming in from libc or something then it doesn't seem worth the cost. =

[PATCH] D86895: [Modules] Add stats to measure performance of building and loading modules.

2020-09-12 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. This looks like a good idea — people will probably have some options on the names and descriptions of the statistics :-) Comment at: clang/lib/Frontend/CompilerInstance.cpp:60 + +ALWAYS_ENABLED_STATISTIC(NumCompileModule, "Number of compiled modules."

[PATCH] D85981: [clang][Modules] Use File Names Instead of inodes As Loaded Module Keys

2020-08-18 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments. Comment at: clang/include/clang/Serialization/ModuleManager.h:62 - /// All loaded modules, indexed by name. - llvm::DenseMap Modules; + /// All loaded modules, indexed by file name. + llvm::StringMap Modules; CodaFi wrote: > a

[PATCH] D85981: [clang][Modules] Use File Names Instead of inodes As Loaded Module Keys

2020-08-18 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments. Comment at: clang/include/clang/Serialization/ModuleManager.h:62 + struct EntryKey { +const FileEntry *Entry; Can you add a doxygen comment explaining why we compute our own hashing as opposed to using the FileEntry pointer?

[PATCH] D85981: [clang][Modules] Use File Names Instead of inodes As Loaded Module Keys

2020-08-18 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments. Comment at: clang/include/clang/Serialization/ModuleManager.h:62 + struct EntryKey { +const FileEntry *Entry; aprantl wrote: > Can you add a doxygen comment explaining why we compute our own hashing as > opposed to using the

[PATCH] D86823: [clang][Modules] Perform an Extra Consistency Check When Searching The ModuleManager's Cache For Implicit Modules

2020-08-28 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments. Comment at: clang/lib/Serialization/ModuleManager.cpp:152 + const FileEntry *Entry) -> bool { +if (Kind != MK_ImplicitModule) { + return true; nit: LLVM style omits curly braces on single

[PATCH] D86823: [clang][Modules] Perform an Extra Consistency Check When Searching The ModuleManager's Cache For Implicit Modules

2020-08-28 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Assuming that it works, this seems fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86823/new/ https://reviews.llvm.org/D86823 ___

[PATCH] D86823: [clang][Modules] Perform an Extra Consistency Check When Searching The ModuleManager's Cache For Implicit Modules

2020-09-02 Thread Adrian Prantl via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG272742a92d24: Perform an extra consistency check when searching ModuleManager's (authored by aprantl). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86823/ne

[PATCH] D88754: [clang] Add a test for CGDebugInfo treatment of blocks

2020-10-05 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. I think that this should either hardcode a 64-bit architecture or it shouldn't test for explicit offsets. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88754/new/ https://reviews.llvm.org/D88754 __

[PATCH] D88754: [clang] Add a test for CGDebugInfo treatment of blocks

2020-10-09 Thread Adrian Prantl via Phabricator via cfe-commits
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/D88754/new/ https://reviews.llvm.org/D88754

[PATCH] D88976: [clang] Use correct address space for global variable debug info

2020-10-09 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. That looks much nicer. Comment at: clang/test/CodeGenHIP/debug-info-address-class.hip:8 + +// CHECK-DAG: ![[FILEVAR0:[0-9]+]] = distinct !DIGlobalVariable(name: "FileVar0"

[PATCH] D88978: [WIP] Attach debug intrinsics to allocas, and use correct address space

2020-10-09 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDecl.cpp:1579 if (EmitDebugInfo && HaveInsertPoint()) { -Address DebugAddr = address; +Address DebugAddr = AllocaAddr.isValid() ? AllocaAddr : address; bool UsePointerValue = NRVO && ReturnValuePointer

[PATCH] D90079: Avoid unnecessary uses of `MDNode::getTemporary`, NFC

2020-10-26 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. The `push_back(nullptr)` reads weird to me, but it is an accurate reflection of what the IR will look like. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90079/new/ https://reviews

[PATCH] D100630: [Debug-Info][DBX] DW_TAG_rvalue_reference_type should not be generated when dwarf version is smaller than 4

2021-05-13 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. Doing this in the backend SGTM, assuming all of @dblaikie's comments are addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100630/new/ https://reviews.llvm.org/D100630 __

[PATCH] D100630: [Debug-Info][DBX] DW_TAG_rvalue_reference_type should not be generated when dwarf version is smaller than 4

2021-05-22 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. If gracefully lowering rvalue references to references needs knowledge about the Clang typesystem (or depends on LLVM IR type uniquing to be efficient, as @dblaikie points out) then we should do it in the Clang frontend. As much as we can we should avoid encoding debug

[PATCH] D94596: [clang][AST] Encapsulate DeclarationNameLoc, NFCI

2021-01-26 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Seem reasonable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94596/new/ https://reviews.llvm.org/D94596 ___

[PATCH] D95704: [CodeGen] Introduce DWARF tag for SwiftTail and emit it in CodeGen.

2021-01-29 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95704/new/ https://reviews.llvm.org/D95704 _

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

2021-09-17 Thread Adrian Prantl via Phabricator via cfe-commits
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] D109541: Increase expected line number for ExtDebugInfo.cpp

2021-09-24 Thread Adrian Prantl via Phabricator via cfe-commits
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] D104082: [CodeGen] Don't create a fake FunctionDecl when generating block/block_byref copy/dispose helper functions

2021-06-22 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl accepted this revision. aprantl added a comment. Block copy helpers don't have any source code associated with them, so as long as they are described by a DISubprogram, the exact format of it most likely does not matter. Comment at: clang/lib/CodeGen/CGBlocks.cpp:1957

[PATCH] D104082: [CodeGen] Don't create a fake FunctionDecl when generating block/block_byref copy/dispose helper functions

2021-06-22 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments. Comment at: clang/lib/CodeGen/CGBlocks.cpp:2144 CGM); // This is necessary to avoid inheriting the previous line number. + StartFunction(GlobalDecl(), ReturnTy, Fn, FI, args); Same here — th

[PATCH] D104883: [CodeGen] Add ParmVarDecls to FunctionDecls that are created to generate ObjC property getter/setter functions

2021-06-24 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. If ` args.push_back(Params[1] = SrcDecl);` doesn't trigger a warning, this seems reasonable. Comment at: clang/lib/CodeGen/CGObjC.cpp:3702 + /*DefArg=*/nullptr); + a

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

2021-07-30 Thread Adrian Prantl via Phabricator via cfe-commits
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] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

2021-08-04 Thread Adrian Prantl via Phabricator via cfe-commits
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] D105564: Fix for DWARF parsing to better handle auto return type for member functions

2021-07-07 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. I think it would be best to split out the Clang change into a separately tested patch. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1694 + // If the declared return type is "auto" we want the linkage name to go + // with the defintion. In ca

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

2021-07-07 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. I think it would be best to split out the Clang change into a separately tested patch. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2693 +if (!try_resolving_type) + return true; + This block looks like

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

2021-07-07 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2693 +if (!try_resolving_type) + return true; + aprantl wrote: > This block looks like it's more complicated than it needs to be. Could you > just say >

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

2021-07-07 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. Could LLDB find the linkage name on the declaration, look that name up in the symbol table, and find the DW_TAG_subprogram DIE for the symbol's start address? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105564/new/ https://reviews.llvm.org/D105564 __

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

2021-10-29 Thread Adrian Prantl via Phabricator via cfe-commits
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] D106585: Fix clang debug info irgen of i128 enums

2021-11-05 Thread Adrian Prantl via Phabricator via cfe-commits
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] D113718: Don't append the working directory to absolute paths

2021-11-11 Thread Adrian Prantl via Phabricator via cfe-commits
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] D113743: [RFC][clang][DebugInfo] Allow function-local statics and types to be scoped within a lexical block

2021-11-15 Thread Adrian Prantl via Phabricator via cfe-commits
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] D116203: [clang] adds unary type transformations as compiler built-ins

2022-08-22 Thread Adrian Prantl via Phabricator via cfe-commits
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] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-23 Thread Adrian Prantl via Phabricator via cfe-commits
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] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

2022-08-24 Thread Adrian Prantl via Phabricator via cfe-commits
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] D136041: [clang][DebugInfo] Emit DISubprogram for extern functions with reserved names

2022-10-17 Thread Adrian Prantl via Phabricator via cfe-commits
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] D137838: [Support] Move TargetParsers to new component

2022-12-20 Thread Adrian Prantl via Phabricator via cfe-commits
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] D137838: [Support] Move TargetParsers to new component

2022-12-20 Thread Adrian Prantl via Phabricator via cfe-commits
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

2022-12-20 Thread Adrian Prantl via Phabricator via cfe-commits
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] D140195: [Clang][CGDebugInfo][ObjC] Mark objc bitfields with the DIFlagBitfield flag

2022-12-20 Thread Adrian Prantl via Phabricator via cfe-commits
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] D142632: [clang][TypePrinter] Support expression template arguments when checking defaultedness

2023-01-26 Thread Adrian Prantl via Phabricator via cfe-commits
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

2023-01-20 Thread Adrian Prantl via Phabricator via cfe-commits
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] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

2022-06-17 Thread Adrian Prantl via Phabricator via cfe-commits
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] D125694: [clang][DebugInfo] Allow function-local statics to be scoped within a lexical block (4/5)

2022-05-26 Thread Adrian Prantl via Phabricator via cfe-commits
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] D125695: [clang][DebugInfo] Allow function-local type-like entities to be scoped within a lexical block (5/5)

2022-05-26 Thread Adrian Prantl via Phabricator via cfe-commits
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] D121100: [clang][DebugInfo] clang should not generate DW_TAG_subprogram entry without DW_AT_name

2022-05-26 Thread Adrian Prantl via Phabricator via cfe-commits
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] D125839: [gmodules] Skip CXXDeductionGuideDecls when visiting FunctionDecls in DebugTypeVisitor

2022-05-31 Thread Adrian Prantl via Phabricator via cfe-commits
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] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

2022-06-01 Thread Adrian Prantl via Phabricator via cfe-commits
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] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

2022-06-01 Thread Adrian Prantl via Phabricator via cfe-commits
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

2022-06-01 Thread Adrian Prantl via Phabricator via cfe-commits
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

2022-06-01 Thread Adrian Prantl via Phabricator via cfe-commits
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

2022-06-01 Thread Adrian Prantl via Phabricator via cfe-commits
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

2022-06-01 Thread Adrian Prantl via Phabricator via cfe-commits
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] D126689: [IR] Enable opaque pointers by default

2022-06-02 Thread Adrian Prantl via Phabricator via cfe-commits
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] D126689: [IR] Enable opaque pointers by default

2022-06-02 Thread Adrian Prantl via Phabricator via cfe-commits
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] D105564: Fix for DWARF parsing to better handle auto return type for member functions

2022-06-07 Thread Adrian Prantl via Phabricator via cfe-commits
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] D157479: [Clang][DebugInfo] Emit narrower base types for structured binding declarations that bind to struct bitfields

2023-08-14 Thread Adrian Prantl via Phabricator via cfe-commits
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] D157479: [Clang][DebugInfo] Emit narrower base types for structured binding declarations that bind to struct bitfields

2023-08-14 Thread Adrian Prantl via Phabricator via cfe-commits
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

2023-08-15 Thread Adrian Prantl via Phabricator via cfe-commits
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] D138597: DebugInfo: Add/support new DW_LANG codes for recent C and C++ versions

2022-11-29 Thread Adrian Prantl via Phabricator via cfe-commits
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] D138597: DebugInfo: Add/support new DW_LANG codes for recent C and C++ versions

2022-12-02 Thread Adrian Prantl via Phabricator via cfe-commits
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] D76377: Correctly initialize the DW_AT_comp_dir attribute of Clang module skeleton CUs

2020-03-20 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl marked an inline comment as done. aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2499 +DIB.createFile(Mod.getModuleName(), TheCU->getDirectory()), +TheCU->getProducer(), false, StringRef(), 0, Mod.getASTFile(), +llvm::

[PATCH] D76377: Correctly initialize the DW_AT_comp_dir attribute of Clang module skeleton CUs

2020-03-20 Thread Adrian Prantl via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG079c6ddaf534: Correctly initialize the DW_AT_comp_dir attribute of Clang module skeleton CUs (authored by aprantl). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D7637

[PATCH] D76385: Allow remapping Clang module include paths

2020-03-20 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl updated this revision to Diff 251789. aprantl added a comment. Rebased and addressed review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76385/new/ https://reviews.llvm.org/D76385 Files: clang/lib/CodeGen/CGDebugInfo.cpp clang/test/Modules/debug-info-moduleimport

[PATCH] D76383: Allow remapping Clang module skeleton CU references with -fdebug-prefix-map

2020-03-20 Thread Adrian Prantl via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG43580a5c5afc: Allow remapping Clang module skeleton CU references with -fdebug-prefix-map (authored by aprantl). Herald ad

[PATCH] D76393: Allow remapping the sysroot with -fdebug-prefix-map.

2020-03-20 Thread Adrian Prantl via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6725c4836a5b: Allow remapping the sysroot with -fdebug-prefix-map. (authored by aprantl). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D76393?vs=251412&id=251798#toc

[PATCH] D76097: improve performance of getSDKName()

2020-03-20 Thread Adrian Prantl via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG0e916bf9f5e4: Driver: Improve performance of getSDKName() (authored by aprantl). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D76393: Allow remapping the sysroot with -fdebug-prefix-map.

2020-03-20 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. Yeah, screwed up during rebasing and accidentally committed an extra file from a different review I reverted and relanded the patch now and the bots are running again. Sorry for the noise. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

<    1   2   3   4   5   6   >