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

aprantl added a comment. I'm afraid I'm going to give up on fixing the AST and return to my debuginfo-only patch. While I was correct in figuring out that ObjCMethodDecl implementations are not linked up as redeclarations of ObjCMethodDecl decls in the interface, that wasn't the whole story:

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

aprantl added a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66121/new/ https://reviews.llvm.org/D66121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67613: [DWARF-5] Support for DWARF-5 C++ language tags

aprantl added inline comments. Comment at: include/clang/AST/DeclCXX.h:2931 +lang_cxx_11 = /* DW_LANG_C_plus_plus_11 */ 0x001a, +lang_cxx_14 = /* DW_LANG_C_plus_plus_14 */ 0x0021 }; SouraVX wrote: > aprantl wrote: > > SouraVX wrote: > > > aprantl

[PATCH] D67613: [DWARF-5] Support for DWARF-5 C++ language tags

aprantl added inline comments. Comment at: include/clang/AST/DeclCXX.h:2931 +lang_cxx_11 = /* DW_LANG_C_plus_plus_11 */ 0x001a, +lang_cxx_14 = /* DW_LANG_C_plus_plus_14 */ 0x0021 }; SouraVX wrote: > aprantl wrote: > > I understand that DWARF does not

[PATCH] D67613: [DWARF-5] Support for DWARF-5 C++ language tags

aprantl added inline comments. Comment at: include/clang/AST/DeclCXX.h:2931 +lang_cxx_11 = /* DW_LANG_C_plus_plus_11 */ 0x001a, +lang_cxx_14 = /* DW_LANG_C_plus_plus_14 */ 0x0021 }; I understand that DWARF does not define a C++17 language constant,

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

aprantl added a comment. In D66121#1656442 , @rjmccall wrote: > Can you prepare an NFC patch with just the changes relating to adding > `ObjCPropertyImplDecl::get{Getter,Setter}MethodDecl`? Sure, I will do that. > I don't get why the redeclaration

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

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

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

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

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

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] D46791: Make -gsplit-dwarf generally available

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] D66121: Debug Info: Nest Objective-C property function decls inside their container.

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

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

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:

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

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

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

aprantl added inline comments. Comment at: test/CodeGen/debug-info-no-location.cpp:4 +// CHECK-NEXT: ret void, !dbg !51 +// CHECK: !48 = distinct !DISubprogram(name: "`dynamic atexit destructor for 'f'", scope: !3, file: !3, line: 16, type: !49, scopeLine: 16, spFlags:

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

aprantl requested changes to this revision. aprantl added inline comments. This revision now requires changes to proceed. Comment at: test/CodeGen/debug-info-no-location.cpp:4 +// CHECK-NEXT: ret void, !dbg !51 +// CHECK: !48 = distinct !DISubprogram(name: "`dynamic atexit

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

aprantl marked an inline comment as done. aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.h:406 + CGBuilderTy , + const ObjCContainerDecl *CD = nullptr); rjmccall wrote: > aprantl wrote:

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

aprantl marked an inline comment as done. aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.h:406 + CGBuilderTy , + const ObjCContainerDecl *CD = nullptr); rjmccall wrote: > Why does this

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

aprantl created this revision. aprantl added reviewers: vsk, rjmccall. aprantl added a project: debug-info. aprantl added a reviewer: davide. This fixes a crash in Clang. Starting with DWARF 5 we are emitting ObjC method declarations as children of their containing entity. This worked for

[PATCH] D65573: Add User docs for ASTImporter

aprantl added a comment. Thanks, the extra documentation is highly appreciated! Comment at: clang/docs/LibASTImporter.rst:19 +``ASTContext`` holds long-lived AST nodes (such as types and decls) that can be referred to throughout the semantic analysis of a file. +There are

[PATCH] D63361: Pretend NRVO variables are references so they can be found by debug info

aprantl added a comment. Thanks, this addresses my concern! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63361/new/ https://reviews.llvm.org/D63361 ___ cfe-commits mailing list

[PATCH] D63361: Pretend NRVO variables are references so they can be found by debug info

aprantl requested changes to this revision. aprantl added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/CodeGen/CGDecl.cpp:1572 + ConvertTypeForMem(getContext().getLValueReferenceType(Ty)); + DebugAddr =

[PATCH] D62622: [CMake] Provide an option to use relative paths in debug info

aprantl added a comment. I like LLVM_USE_RELATIVE_PATHS_IN_DEBUG_INFO. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62622/new/ https://reviews.llvm.org/D62622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D62622: [CMake] Provide an option to use relative paths in debug info

aprantl added a comment. -DLLVM_USE_RELATIVE_PATHS is very generic sounding and might give a false impression. Is there a name that emphasizes that this affects the debug info of the built compiler? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62622/new/

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Couple more nitpicks, but otherwise good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58035/new/ https://reviews.llvm.org/D58035

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

aprantl added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:4468 } +static void analyzeParametersModification( /// Analyzes each function parameter to determine whether it is constant throughout the function body. Comment at:

[PATCH] D58033: Add option for emitting dbg info for call site parameters

aprantl accepted this revision. aprantl added a comment. LGTM, with an additional testcase. Comment at: test/CodeGenCXX/dbg-info-all-calls-described.cpp:23 +// RUN: | FileCheck %s -check-prefix=HAS-ATTR \ +// RUN: -implicit-check-not=DIFlagAllCallsDescribed +

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

aprantl added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:4537 + CGM.getLangOpts().Optimize) { +for (auto : DeclCache) { + auto *D = SP.first; djtodoro wrote: > aprantl wrote: > > djtodoro wrote: > > > aprantl wrote: > > > > Just

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

aprantl added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:4537 + CGM.getLangOpts().Optimize) { +for (auto : DeclCache) { + auto *D = SP.first; djtodoro wrote: > aprantl wrote: > > Just looking at the type declarations in

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

aprantl accepted this revision. aprantl added a comment. Alright, thanks for taking the time to walk me through the thought process! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61438/new/ https://reviews.llvm.org/D61438

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

aprantl added inline comments. Comment at: clang/include/clang/AST/ASTImporter.h:203 /// context, or the import error. -llvm::Expected Import_New(TypeSourceInfo *FromTSI); -// FIXME: Remove this version. -TypeSourceInfo *Import(TypeSourceInfo *FromTSI); +

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

aprantl added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:3885 + if (ArgNo) { +auto *PD = dyn_cast(VD); +ParmCache[PD].reset(D); A `dyn_cast` followed by an unconditional use seems strange. I would expect either a `cast` or an `if (PD)`

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

aprantl added inline comments. Comment at: clang/include/clang/AST/ASTImporter.h:203 /// context, or the import error. -llvm::Expected Import_New(TypeSourceInfo *FromTSI); -// FIXME: Remove this version. -TypeSourceInfo *Import(TypeSourceInfo *FromTSI); +

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

aprantl added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:5039 + if (!ToOrErr) +// FIXME: return the error? +consumeError(ToOrErr.takeError()); We don't typically commit FIXME's into LLVM code. Why not just deal

[PATCH] D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Could we test this by doing -dump-ast of From and To and FileCheck-ing the output? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61140/new/ https://reviews.llvm.org/D61140

[PATCH] D61079: Skip type units/type uniquing when we know we're only emitting the type once (vtable-based emission when triggered by a strong vtable, with -fno-standalone-debug)

aprantl added a comment. I hope I'm getting this right, but if I remember correctly, the significant part in this test is what is a FwdDecl as opposed to a definition. The identifiers no longer make it into the debug info and you should see no difference in the produced binaries with this

[PATCH] D58033: Add option for emitting dbg info for call site parameters

aprantl added a comment. Is there some kind of testcase? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/ https://reviews.llvm.org/D58033 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D58033: Add option for emitting dbg info for call sites

aprantl added a comment. And if we plan to enable it by default, too, perhaps not adding a driver option (CC1 only) is preferable, since we need to maintain compatibility with driver options indefinitely. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

aprantl added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:4537 + CGM.getLangOpts().Optimize) { +for (auto : DeclCache) { + auto *D = SP.first; Just looking at the type declarations in CGDebugInfo.h: Why not iterate over the `SPCache`

[PATCH] D58033: Add option for emitting dbg info for call sites

aprantl added inline comments. Comment at: include/clang/Driver/Options.td:919 HelpText<"Do not use jump tables for lowering switches">; +def emit_param_entry_values : Joined<["-"], "femit-param-entry-values">, + Group, I assume that

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Okay, so it looks like dexter might be a replacement for the lldgb.py wrapper and would support gdb, lldb, and the visual studio debugger. I think it would be nice to migrate the bulk of

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

aprantl added a comment. Did anyone take the time to look at dexter and whether it would fit the bill here? It would be great to avoid reimplementing its functionality just to have something that then only works on Windows. My position is that for the kind of very basic end-to-end testing that

[PATCH] D60238: Verify that Android targets generate DWARF 4 by default.

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

[PATCH] D60238: Verify that Android targets generate DWARF 4 by default.

aprantl added inline comments. Comment at: clang/test/Driver/debug-options.c:280 // G_STANDALONE: "-debug-info-kind=standalone" +// G_DWARF2: "-dwarf-version=2" // G_DWARF4: "-dwarf-version=4" What's that for? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D49863: [istream] Fix error flags and exceptions propagated from input stream operations

aprantl added a comment. It looks like this may have broken some LLDB data formatters: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22934/

[PATCH] D54978: Move the SMT API to LLVM

aprantl added a comment. Thanks! The problem with these methods is that LLVM_DUMP_METHOD forces the function to be emitted even if it is not used and Clang modules without local submodule visibility will implicitly include all header files in a module, which will then cause the missing symbol

[PATCH] D54978: Move the SMT API to LLVM

aprantl added a comment. I'm afraid this broke some bots that build with `LLVM_ENABLE_MODULES=1`. For example: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22411/consoleFull#710926295dd1929ea-7054-4089-b7ef-4624c3781fa4 Undefined symbols for architecture x86_64:

[PATCH] D59197: [NFC][clang][PCH][ObjC] Add some missing `VisitStmt(S); `

aprantl added a comment. How can this change be NFC? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59197/new/ https://reviews.llvm.org/D59197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D58992: [CUDA][HIP][DebugInfo] Skip reference device function

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1755 +} +V = V->stripPointerCasts(); } hliao wrote: > aprantl wrote: > > This

[PATCH] D58992: [CUDA][HIP][DebugInfo] Skip reference device function

aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1755 +} +V = V->stripPointerCasts(); } This wasn't there before... why is this necessary? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D57976: -gmodules: Don't emit incomplete breadcrumbs pointing to nonexistant PCM files.

aprantl added a comment. In D57976#1393939 , @dblaikie wrote: > This, I guess, is part of the impact of moving towards explicit modules > (-fmodule-name is for building a module with that name, right?)? That option is overloaded. It's used to specify

[PATCH] D57976: -gmodules: Don't emit incomplete breadcrumbs pointing to nonexistant PCM files.

aprantl created this revision. aprantl added reviewers: JDevlieghere, bruno. When a module name is specified as -fmodule-name, that module gets a clang::Module object, but it won't actually be built or imported; it will be textual. CGDebugInfo wouldn't detect this and them emit a DICompileUnit

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

aprantl added inline comments. Comment at: lib/AST/ASTImporter.cpp:1672 // FIXME: Handle this case somehow better. - consumeError(ImportedOrErr.takeError()); + else +consumeError(ImportedOrErr.takeError()); this else is redundant.

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

aprantl added a comment. Thanks, should be fixed in r348610! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55085/new/ https://reviews.llvm.org/D55085 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

aprantl updated this revision to Diff 176677. aprantl added a reviewer: ilya-biryukov. aprantl added a comment. Ilya, this updated revision should restore the original GCOV behavior both for absolute and relative paths. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55085/new/

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

aprantl marked an inline comment as done. aprantl added inline comments. Comment at: test/CodeGenCXX/debug-prefix-map-lambda.cpp:7 + // CHECK: !DISubprogram(name: "b<(lambda at + // CHECK-SAME: /SOURCE_ROOT/debug-prefix-map-lambda.cpp + // CHECK-SAME:

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

aprantl updated this revision to Diff 176194. aprantl added a comment. Only initialize `RemapFilePaths` when `DebugPrefixMap` is nonempty. Should be slightly faster. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55137/new/ https://reviews.llvm.org/D55137 Files:

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

aprantl added a comment. In D55137#1315133 , @dblaikie wrote: > Can't say I know much abouth the path remapping functionality - what it's > used for, where it's implemented in general, etc - so figure someone with > more of that knowledge might be best

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

aprantl updated this revision to Diff 176174. aprantl added a comment. fix a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55137/new/ https://reviews.llvm.org/D55137 Files: include/clang/AST/PrettyPrinter.h lib/AST/TypePrinter.cpp lib/CodeGen/CGDebugInfo.cpp

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

aprantl created this revision. aprantl added reviewers: dblaikie, davide. aprantl added a project: debug-info. This adds a callback to `PrintingPolicy` to allow `CGDebugInfo` to remap file paths according to `-fdebug-prefix-map`. Otherwise the debug info (particularly function names for C++

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

aprantl marked an inline comment as done. aprantl added inline comments. Comment at: lib/IR/DiagnosticInfo.cpp:39 #include "llvm/Support/ScopedPrinter.h" +#include "llvm/Support/raw_ostream.h" #include probinson wrote: > Do we use a case-sensitive sort of

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

aprantl added a comment. Adding a few more reviewers since I'm touching the backend diagnostics. The backend change is supposed to be NFC, but it never hurts to have more feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55085/new/ https://reviews.llvm.org/D55085

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

aprantl updated this revision to Diff 176152. aprantl added a comment. Remove debugging code accidentally left in the patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55085/new/ https://reviews.llvm.org/D55085 Files: include/llvm/IR/DiagnosticInfo.h

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

aprantl updated this revision to Diff 176148. aprantl added a reviewer: davide. aprantl added a comment. Herald added subscribers: nhaehnle, jvesely. Turns out that my patch had an unintended interaction with the backend diagnostics engine. This is an updated version of the patch that also

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

aprantl updated this revision to Diff 175982. aprantl added a comment. Bugfix for LexicalBlockFiles + testcase updates. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55085/new/ https://reviews.llvm.org/D55085 Files: lib/CodeGen/CGDebugInfo.cpp test/CodeGen/debug-info-abspath.c

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

aprantl created this revision. aprantl added reviewers: dblaikie, probinson. aprantl added a project: debug-info. As discussed on llvm-dev today, Clang currently emits redundant directories in DIFile entries, such as `.file 1 "/Volumes/Data/llvm"

[PATCH] D55037: [-gmodules] Honor -fdebug-prefix-map in the debug info inside PCMs.

aprantl created this revision. aprantl added reviewers: bruno, rsmith. This patch passes -fdebug-prefix-map (a feature for renaming source paths in the debug info) through to the per-module codegen options and adds the debug prefix map to the module hash. rdar://problem/46045865

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

aprantl added a comment. In D44100#1302478 , @a_sidorin wrote: > I don't mean only review. As I guess, you guys have MacOS machines so you can > check if the problem is still present in the updated version. FYI, typically problems with ASTImporter in

[PATCH] D54756: [DebugInfo] NFC Clang test changes for DISubprogram flags

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Everybody with out-of-tree tests will be excited ;-) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54756/new/ https://reviews.llvm.org/D54756

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

aprantl added a comment. In https://reviews.llvm.org/D54187#1290317, @zturner wrote: > In https://reviews.llvm.org/D54187#1290297, @aprantl wrote: > > > In https://reviews.llvm.org/D54187#1290293, @zturner wrote: > > > > > Especially since as far as I can tell, nobody has even run > > >

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

aprantl added a comment. In https://reviews.llvm.org/D54187#1290298, @probinson wrote: > In https://reviews.llvm.org/D54187#1290294, @zturner wrote: > > > In https://reviews.llvm.org/D54187#1290282, @probinson wrote: > > > > > +gbedwell > > > > > > Just to throw the idea out there, why not

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

aprantl added a comment. In https://reviews.llvm.org/D54187#1290293, @zturner wrote: > Especially since as far as I can tell, nobody has even run debuginfo-tests > since late August, because it was actually broken by r341135 on August 30 > (fixed in r346060 yesterday) Can you please refrain

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

aprantl added a comment. > I think the only way to realistically make this work for all platforms would > be to separate the source file from the input/output. The source file would > be the test case, and if you wanted to support a different debugger you would > need to supply a different

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

aprantl added a comment. It would be nice if instead of having a set of windows-only tests, we could wrap cdb similar to llgdb.py wraps LLDB, so these tests run on all platforms. If the Windows debugger is just too different for this to make sense, let me know.

[PATCH] D45045: [DebugInfo] Generate debug information for labels.

aprantl added a comment. In https://reviews.llvm.org/D45045#1270442, @HsiangKai wrote: > In https://reviews.llvm.org/D45045#1247427, @vitalybuka wrote: > > > Reverted in r343183 > > https://bugs.llvm.org/show_bug.cgi?id=39094 > > > >

[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

aprantl updated this revision to Diff 170434. aprantl added a comment. further simplify testcase https://reviews.llvm.org/D53459 Files: lib/CodeGen/CGExpr.cpp test/CodeGenCXX/ubsan-check-debuglocs.cpp Index: test/CodeGenCXX/ubsan-check-debuglocs.cpp

[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

aprantl updated this revision to Diff 170433. aprantl added a comment. Simplify testcase https://reviews.llvm.org/D53459 Files: lib/CodeGen/CGExpr.cpp test/CodeGenCXX/ubsan-check-debuglocs.cpp Index: test/CodeGenCXX/ubsan-check-debuglocs.cpp

[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

aprantl added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:2871 + auto *DI = CGF.getDebugInfo(); + SourceLocation Loc = DI ? DI->getLocation() : SourceLocation(); + auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc); vsk wrote: > Why

[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

aprantl created this revision. aprantl added a reviewer: vsk. Function calls without a !dbg location inside a function that has a DISubprogram make it impossible to construct inline information and are rejected by the verifier. This patch ensures that sanitizer check function calls have a !dbg

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

aprantl added a comment. RTTI? Repository: rC Clang https://reviews.llvm.org/D38061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

aprantl added a comment. Oh wait, this patch is just for dumping the ASTs? Can you elaborate why this makes it into a binary then? Repository: rC Clang https://reviews.llvm.org/D38061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

aprantl added a comment. I have a vague recollection that this column info hack was added to disambiguate two types defined on the same line (which is something that happened more often than one would think because of macro expansion). Did you do the git archeology to ensure that the original

[PATCH] D53329: Generate DIFile with main program if source is not available

aprantl added a comment. Couldn't you just pass `-main-file-name` to cc1 instead? Repository: rC Clang https://reviews.llvm.org/D53329 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51910: [Modules] Add platform feature to requires clause

aprantl added inline comments. Comment at: lib/Basic/Module.cpp:101 + // variant (2), the simulator is hardcoded as part of the platform name. Both + // forms above should match "iossimulator" and "ios-simulator" features. + if (Target.getTriple().isOSDarwin() &&

[PATCH] D51910: [Modules] Add platform feature to requires clause

aprantl added inline comments. Comment at: docs/Modules.rst:470 +*platform variant* + A specific os/platform variant (e.g. ``ios``, ``macos``, ``android``, ``win32``, ``linux``, etc) is available. aprantl wrote: > Does this work with platforms+environment

[PATCH] D51990: [DebugInfo] Fix emitting of bit offset for ObjC

aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2369 + ? CGM.getObjCRuntime().ComputeBitfieldBitOffset(CGM, ID, Field) + : 0; } else { JDevlieghere wrote: > JDevlieghere wrote: > > aprantl wrote: > > >

[PATCH] D51990: [DebugInfo] Fix emitting of bit offset for ObjC

aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2369 + ? CGM.getObjCRuntime().ComputeBitfieldBitOffset(CGM, ID, Field) + : 0; } else { aprantl wrote: > It might help to attempt some git blame

[PATCH] D51990: [DebugInfo] Fix emitting of bit offset for ObjC

aprantl requested changes to this revision. aprantl added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2369 + ? CGM.getObjCRuntime().ComputeBitfieldBitOffset(CGM, ID, Field) + : 0; }

[PATCH] D51910: [Modules] Add platform feature to requires clause

aprantl added inline comments. Comment at: docs/Modules.rst:470 +*platform variant* + A specific os/platform variant (e.g. ``ios``, ``macos``, ``android``, ``win32``, ``linux``, etc) is available. Does this work with platforms+environment combinations, such

[PATCH] D51807: Remove all uses of DIFlagBlockByrefStruct from Clang

aprantl added a comment. ( https://reviews.llvm.org/D51763 is not *really* a dependency, but it's closely related. ) https://reviews.llvm.org/D51807 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51807: Remove all uses of DIFlagBlockByrefStruct from Clang

aprantl created this revision. aprantl added a reviewer: debug-info. Herald added a subscriber: JDevlieghere. This patch removes the last reason why DIFlagBlockByrefStruct from Clang by directly implementing the drilling into the member type done in DwarfDebug::DbgVariable::getType() into the

[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)

aprantl added a comment. In https://reviews.llvm.org/D51576#1223562, @labath wrote: > The interactions here are a bit weird, but the short answer is no, this will > not affect apple tables in any way. Then I have no problem with this patch :-) Repository: rC Clang

[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)

aprantl added a comment. This is DWARF5+ only, right? (We shouldn't change the preference of Apple accelerator tables for DWARF 4 and earlier). Repository: rC Clang https://reviews.llvm.org/D51576 ___ cfe-commits mailing list

[PATCH] D51393: Provide a method for generating deterministic IDs for pointers allocated in BumpPtrAllocator

aprantl added a comment. Just for consideration: The raw pointers in dumps are sometimes useful while in a debugger session, because you can cast a pointer and dump the object in the debugger. https://reviews.llvm.org/D51393 ___ cfe-commits

[PATCH] D51393: Provide a method for generating deterministic IDs for pointers allocated in BumpPtrAllocator

aprantl added inline comments. Comment at: llvm/include/llvm/Support/Allocator.h:292 +const char *P = reinterpret_cast(Ptr); +for (size_t Idx=0; Idx < Slabs.size(); Idx++) { + const char *S = reinterpret_cast(Slabs[Idx]); clang-format?

[PATCH] D50870: Close FileEntries of cached files in ModuleManager::addModule().

aprantl added a comment. @bruno: When we last discussed this my plan was to avoid the stat() in lookupModuleFile() for files that were just added to the PCMCache by WriteAST() entirely, but ModuleManager::Modules is a DenseMap and lookupModuleFile() is the easiest way to create a new

[PATCH] D50870: Close FileEntries of cached files in ModuleManager::addModule().

aprantl created this revision. aprantl added reviewers: bruno, rsmith, teemperor. Herald added a subscriber: llvm-commits. Close FileEntries of cached files in ModuleManager::addModule(). While investigating why LLDB (which can build hundreds of clang modules during one debug session) was

[PATCH] D50122: Complex Variable defined in InitCapture Crash fix

aprantl added inline comments. Comment at: test/SemaCXX/lambda-init-capture-vardefine.cpp:3 +// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s +// expected-no-diagnostics + These kinds of tests that don't check for any output are a bit dangerous, because

[PATCH] D50089: [DWARF v4] Suppressing the __debug_ranges section when there are no ranges

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: apple-accel.cpp:11 +__attribute__((section("1,__text_foo"))) void foo() {} int main (int argc, char const *argv[]) { return argc; } A

[PATCH] D48241: [DebugInfo] Emit ObjC methods as part of interface.

aprantl added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:4239 +// Add methods to interface. +for (auto p : ObjCMethodCache) { + if (p.second.empty()) JDevlieghere wrote: > dexonsmith wrote: > > Some comment for "p" here. > Fixed; I'll

[PATCH] D48241: [DebugInfo] Emit ObjC methods as part of interface.

aprantl added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:4246 + if (it == TypeCache.end()) +continue; + What does it mean that a type is not being found in the cache here? Comment at: lib/CodeGen/CGDebugInfo.h:102 +

<    1   2   3   4   5   6   >