[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

This revision was automatically updated to reflect the committed changes. Closed by commit rG34b9b1ea4874: Disable -Wmissing-prototypes for internal linkage functions that aren't… (authored by dblaikie). Changed prior to commit: https://reviews.llvm.org/D121328?vs=417989&id=418360#toc Reposito

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie added a comment. @aaron.ballman wouldn't mind your take on this to see if this seems sufficiently robust, tested, etc. (should I move the isExternallyVisible check even further down? So its side effects are even less impactful (maybe there are other warnings that care about this sort o

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie added a comment. In D121328#3405623 , @deansturtevant wrote: > Aha! (I think). > If the code to test "isExternalVisible" is executed *after* the code to test > whether it's a C++ member function (the very next test), then the problem > wouldn'

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie updated this revision to Diff 417989. dblaikie added a comment. Add regression test for the -Wnon-c-typedef-for-linkage issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121328/new/ https://reviews.llvm.org/D121328 Files: clang/lib/Se

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie updated this revision to Diff 417985. dblaikie added a comment. Try Dean's suggestion of moving this check further down Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121328/new/ https://reviews.llvm.org/D121328 Files: clang/lib/Sema/Sem

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

dblaikie added a comment. is this an issue for Clang Header Modules codegen too? (maybe the solution should be the same for both, then) & maybe we should do this regardless of the presence/absence/type of initializer, just for consistency? CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

dblaikie added a comment. In D119409#3398483 , @ChuanqiXu wrote: > In D119409#3396690 , @dblaikie > wrote: > >> Not even necessarily then - if you have code like protobufs (large amounts >> of autogenerated code

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

dblaikie added a comment. SOrry, I don't have much context here - the more informative (module/internal linkage) diagnostic does seem better to me than saying "is not exported", even if it's a bit esoteric for some users. We do have other diagnostics that mention linkage, I'm sure (because it's

[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

dblaikie added inline comments. Comment at: clang/test/CodeGenCXX/inconsistent-export-template.cpp:1-11 +// RUN: %clang_cc1 -std=c++20 %s -S -emit-llvm -triple %itanium_abi_triple -disable-llvm-passes -o - | FileCheck %s + +export module m; +export template +void f() { +} + ---

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie added a comment. In D121328#3396984 , @aaron.ballman wrote: >> once we figure out what to do about the change in behavior for >> -Wnon-c-typedef-for-linkage > > The devil is in the details; I'm not sure what to do here. I don't think > there's

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121328/new/ https://reviews.llvm.org/D121328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D122046: [clang] Remove Address::deprecated from MveEmitter

dblaikie added inline comments. Comment at: clang/utils/TableGen/MveEmitter.cpp:1197 +const Type *Ty = nullptr; +if (auto *DI = dyn_cast(D->getArg(0))->getOperator()) + if (auto *PTy = dyn_cast(getType(DI, Param))) aeubanks wrote: > simon_tatham wrot

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

dblaikie added a comment. In D119409#3334537 , @ChuanqiXu wrote: > In D119409#3332313 , @dblaikie > wrote: > >> (maybe relevant: For what it's worth: I originally implemented inline >> function homing in modules

[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

dblaikie added inline comments. Comment at: clang/test/Modules/inconsist-export-template.cpp:1-2 +// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify +// expected-no-diagnostics +export module m; ChuanqiXu wrote: > urnathan wrote: > > dblaikie wrote: > > > Thi

[PATCH] D122046: [clang] Remove Address::deprecated from MveEmitter

dblaikie added inline comments. Comment at: clang/utils/TableGen/MveEmitter.cpp:1197 +const Type *Ty = nullptr; +if (auto *DI = dyn_cast(D->getArg(0))->getOperator()) + if (auto *PTy = dyn_cast(getType(DI, Param))) nikic wrote: > Should be either `ca

[PATCH] D120610: [DebugInfo] Include DW_TAG_skeleton_unit when looking for parent UnitDie

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Thanks, sounds good! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120610/new/ https://reviews.llvm.org/D120610 ___

[PATCH] D120610: [DebugInfo] Include DW_TAG_skeleton_unit when looking for parent UnitDie

dblaikie added a comment. It'd be good to include some testing beyond "does not crash" - like what was the specific debug info we were trying to create when the crash was hit? Perhaps we should be testing that (since the crash demonstrates we weren't testing that anywhere else) Repository:

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

dblaikie added a comment. In D118511#3373173 , @tstellar wrote: > In D118511#3373082 , @dblaikie > wrote: > >> In D118511#3372728 , @jyknight >> wrote: >> >>> In D118511

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

dblaikie added a comment. In D118511#3372728 , @jyknight wrote: > In D118511#3371432 , @tstellar > wrote: > >> I'm fine with reverting if you think this is the best solution. I just >> would like to conclude so

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

dblaikie added a comment. Yeah, happy to hear other perspectives - but my rough reaction is similar: putting mangled names in the "name" field seems problematic (consumer wouldn't necessarily know that the name should be demangled, for instance? Maybe?). So at the IR level maybe it's better to

[PATCH] D120610: [DebugInfo] Include DW_TAG_skeleton_unit when looking for parent UnitDie

dblaikie added a comment. Fixes in LLVM require tests in LLVM - probably taking the clang test and compiling that to llvm IR (include the original C++ source in a comment in the IR test case) and then testing it in LLVM instead of clang. Also looks like the test could be simplified a bit more:

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie created this revision. dblaikie added reviewers: aaron.ballman, rsmith, denik, deansturtevant. Herald added a project: All. dblaikie requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Some functions can end up non-externally visible d

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

dblaikie added a comment. In D118511#3369114 , @tbaeder wrote: > Hey @dblaikie, seems like this has never been pushed? Yeah, was holding off on this because it looked like maybe there was still outstanding work on the nuance/precise nature of the ABI ch

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

dblaikie added a comment. In D119051#3350072 , @rnk wrote: > I would structure this as a LangOpt, and feed the target checks and ABI > compat checks into the default setting for it. It could be something like > `DefaultedSMFArePOD` / `-f[no-]defaulted-s

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

dblaikie updated this revision to Diff 413641. dblaikie added a comment. Herald added subscribers: dexonsmith, dang. Herald added a project: All. Add a -f driver flag for defaulted-smf-are-pod Include the other cases from itanium ABI issue 66 (implicitly and explicitly deleted special members, an

[PATCH] D121077: Simplify OpenMP Lambda use

This revision was automatically updated to reflect the committed changes. Closed by commit rGc0a6433f2b51: Simplify OpenMP Lambda use (authored by dblaikie). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121077/new/ https://reviews.llvm.org/D121077

[PATCH] D120989: Support debug info for alias variable

dblaikie added subscribers: JDevlieghere, aprantl. dblaikie added a comment. Maybe APple folks would want to move forward with this only when targeting lldb, though - which might unblock this patch. @aprantl @JDevlieghere ? What do you folks think? Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D120989: Support debug info for alias variable

dblaikie added a comment. In D120989#3362912 , @kavitha-natarajan wrote: > In D120989#3362491 , @dblaikie > wrote: > >> In D120989#3362490 , @dblaikie >> wrote: >> >>>

[PATCH] D121077: Simplify OpenMP Lambda use

dblaikie created this revision. dblaikie added a reviewer: ABataev. Herald added subscribers: guansong, yaxunl. Herald added a project: All. dblaikie requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang

[PATCH] D120989: Support debug info for alias variable

dblaikie added a comment. In D120989#3362490 , @dblaikie wrote: > Broad question about debug info for aliases: How's this proposed solution > compare/contrast to GCC's behavior? Aaand, I see that was described in thhe patch description. Sorry for not r

[PATCH] D120989: Support debug info for alias variable

dblaikie added a comment. Broad question about debug info for aliases: How's this proposed solution compare/contrast to GCC's behavior? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120989/new/ https://reviews.llvm.org/D120989 ___

[PATCH] D120989: Support debug info for alias variable

dblaikie added a comment. I think it'd potentially go somewhere in `DWARFDebugLine::LineTable::getFileLineInfoForAddress` for instance - which could inspect the candidate row in the line table, and if it's line zero, it could go back one row in the table. This would avoid probing addresses that

[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

dblaikie added inline comments. Comment at: clang/test/Modules/inconsist-export-template.cpp:19-23 +// FIXME: We should reject following specialization, +// since it tries to export a name which is already introduced. +export template <> +void f1() { + urnathan w

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

dblaikie added inline comments. Comment at: clang/test/Modules/inconsist-export-template.cpp:1-2 +// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify +// expected-no-diagnostics +export module m; This test doesn't appear to test anything - it verifies that thi

[PATCH] D102122: Support warn_unused_result on typedefs

dblaikie added a comment. I don't recall all the context, but did try discussing this with the committee folks and got a variety of strong opinions/wasn't sure whether there was a path forward: https://lists.isocpp.org/ext/2021/05/index.php#msg16554 (for those with access to that). What's your

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

dblaikie added a subscriber: shafik. dblaikie added a comment. In D113743#3177437 , @krisb wrote: > In D113743#3175301 , @dblaikie > wrote: > >> Not super surprising that lldb might not be able to deal with DWARF

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

dblaikie added a comment. In D119409#3332476 , @urnathan wrote: > In D119409#3332313 , @dblaikie > wrote: > >> > > That's interesting data. I guess one could emit the out-of-line bodies into > their own sectio

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

dblaikie added a comment. (maybe relevant: For what it's worth: I originally implemented inline function homing in modules codegen for Clang Header Modules - the results I got for object file size in an -O0 build were marginal - a /slight/ win in object file size, but not as much as we might've

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

dblaikie added inline comments. Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global -// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global +// CHECK-DAG: @inline_var_e

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

dblaikie added a comment. In D105169#3315009 , @MaskRay wrote: > It may not be worth changing now, but I want to mention: it's more > conventional to have a `BoolOption` which adds `-[no-]noundef-analysis`. > Since both positive and negative forms exist

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

dblaikie added a comment. In D119051#3315747 , @rjmccall wrote: > And this should be raised as an Itanium issue. Ah, looks like this is the existing https://github.com/itanium-cxx-abi/cxx-abi/issues/66 Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

dblaikie added a comment. Thanks for chiming in! In D119051#3315741 , @rjmccall wrote: > Changing the C++03 POD definition is going to be substantially ABI-breaking > at this point. Any logic to change it needs to be platform-specific. Even if it's on

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

dblaikie added a comment. In D119051#3314138 , @Bhramar.vatsa wrote: > Sorry, but I can only add a bit more confusion: > https://godbolt.org/z/fT19KTh34 > There are two cases, only differing in terms of user-defined constructor. > > Gcc and clang differ

[PATCH] D119051: Fix pod-packed functionality to use the C++11 definition of pod-ness

dblaikie added a comment. Yeah, looks like GCC believes `cxx11_pod::t1` to be C++03 POD even though it doesn't quite follow the letter of the standard - but since teh "= default" is an extension, I guess they get to define what that extension means. Here's an example: struct t1 { int i; };

[PATCH] D119051: Fix pod-packed functionality to use the C++11 definition of pod-ness

dblaikie updated this revision to Diff 406904. dblaikie added a comment. - Change the definition of C++03 POD to include defaulted functions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 Files: clang/

[PATCH] D119051: Fix pod-packed functionality to use the C++11 definition of pod-ness

dblaikie added a comment. In D119051#3302125 , @rnk wrote: > Looks like the test fails on the Windows pre-merge bot: > https://buildkite.com/llvm-project/premerge-checks/builds/77696#1836f181-a998-4695-b587-a83239ea > > The debian bot seems to be fai

[PATCH] D119051: Fix pod-packed functionality to use the C++11 definition of pod-ness

dblaikie updated this revision to Diff 406628. dblaikie added a comment. Remove FieldClass check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 Files: clang/lib/AST/RecordLayoutBuilder.cpp clang/test

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

dblaikie marked an inline comment as done. dblaikie added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:2029-2036 // The align if the field is not packed. This is to check if the attribute // was unnecessary (-Wpacked). CharUnits UnpackedFieldAlign =

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

dblaikie updated this revision to Diff 406626. dblaikie added a comment. Refactor the max/min/preferred alignment calculations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118511/new/ https://reviews.llvm.org/D118511 Files: clang/include/clang

[PATCH] D119051: Fix pod-packed functionality to use the C++11 definition of pod-ness

dblaikie added a comment. In D119051#3298491 , @dblaikie wrote: > Test case showing GCC's behavior here: https://godbolt.org/z/4W7j8Yd54 Oh, and demonstrating this applies (as best as I can figure) even when compiling in C++03 mode: https://godbolt.org/

[PATCH] D119051: Fix pod-packed functionality to use the C++11 definition of pod-ness

dblaikie added a comment. Test case showing GCC's behavior here: https://godbolt.org/z/4W7j8Yd54 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 ___ cfe-commits

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

dblaikie added a comment. Hmm, I guess it might be the C++11 definition, as suggested - since a base class (even a public one) seems to classify the type as "non pod" as far as GCC is concerned ( In D117616#3298001 , @dblaikie wrote: > In D117616#32958

[PATCH] D119051: Fix pod-packed functionality to use the C++11 definition of pod-ness

dblaikie created this revision. dblaikie added reviewers: rsmith, Bhramar.vatsa, rjmccall, rnk. dblaikie requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D119051 Files: clang

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

dblaikie added a comment. In D117616#3295859 , @Bhramar.vatsa wrote: > @dblaikie > The condition `FieldClass->isPOD()` returns false for the following case > (when considering field 'struct foo t' of 'struct foo1') : > > class foo { > foo() = de

[PATCH] D118781: Reduce dependencies on llvm/BinaryFormat/Dwarf.h

dblaikie added a comment. after: 10978519 before: 11245451 Doesn't appear to be a /huge/ win to me... but I don't mind too much. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118781/new/ https://reviews.llvm.org/D118781 __

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

dblaikie updated this revision to Diff 405468. dblaikie added a comment. - Create a separate sub-warning flag of -Wpacked (-Wpacked-non-pod), in case you're just interested in the ABI breaks here - Warn only when this produces a different alignment requirement for the field Repository: rG LLV

[PATCH] D118762: Remove -Wweak-template-vtables

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

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

dblaikie added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1895 + Target.isPS4() || Target.isOSDarwin())) || + D->hasAttr(); dblaikie wrote: > rsmith wrote: > > Would it make sense to wa

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

dblaikie created this revision. dblaikie added a reviewer: rsmith. dblaikie requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D118511 Files: clang/include/clang/Basic/Diagnost

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG277123376ce0: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs (authored by dblaikie). Repository: rG LLVM Github Mo

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

dblaikie added a comment. Will wait on this 'til next Monday at least. Wouldn't mind a double-check/confirmation from @rjmccall that the Darwin checking is suitable (since I didn't see any other Darwin checking in RecordLayoutBuilder). Comment at: clang/lib/AST/RecordLayoutBu

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

dblaikie added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1891 + llvm::Triple Target = Context.getTargetInfo().getTriple(); + bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() || + Context.getLangOpts().get

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

dblaikie updated this revision to Diff 403825. dblaikie marked 3 inline comments as done. dblaikie added a comment. rsmith's fixes for release notes added some more test coverage to demonstrate that other fields are still packed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

dblaikie added a comment. I didn't find any mention of Darwin in RecordLayoutBuilder, which surprised me - I thought maybe Darwin was using the ClangABICompat to express its ABI compatibility requirements (which would sort of be handy rather than having to test multiple target features, for tho

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

dblaikie updated this revision to Diff 403449. dblaikie added a comment. Herald added a subscriber: dexonsmith. Relnotes Opt out PS4 and Darwin Add support for -fclang-abi-compat=13 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117616/new/ https://

[PATCH] D117744: [Driver] Remove obsoleted -gz=zlib-gnu

dblaikie added a comment. In D117744#3273481 , @MaskRay wrote: > In D117744#3273463 , @dblaikie > wrote: > >> I don't especially feel that removing low-cost features like this is a great >> idea (the benefit to

[PATCH] D117744: [Driver] Remove obsoleted -gz=zlib-gnu

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. I don't especially feel that removing low-cost features like this is a great idea (the benefit to cleaning up a small amount of code compared to the (albeit small) risk of breaking users d

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

dblaikie added a subscriber: Xiangling_L. dblaikie added a comment. @Xiangling_L - I see you made an ABI change for AIX in rG05ad8e942996f36cc694478542ccd84aa5bbb80f - any idea if this new ABI break/fix is relevant to AIX? Wo

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

dblaikie added a subscriber: probinson. dblaikie added a comment. @probinson for Sony ABI input Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117616/new/ https://reviews.llvm.org/D117616 ___ cfe-commits

[PATCH] D45438: [CodeView] Enable debugging of captured variables within C++ lambdas

dblaikie added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:812-814 + // CodeView types with C++ mangling need a type identifier. + if (CGM.getCodeGenOpts().EmitCodeView) +return true; rnk wrote: > dblaikie wrote: > > bwyma wrote: > > > dblaikie

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

dblaikie added a comment. In D117616#3270205 , @tstellar wrote: > So to summarize: this will change the ABI of clang built objects from clang13 > -> clang14 in order to make clang14 compatible with gcc? That's the idea, yes. (only for non-pod members of

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117616/new/ https://reviews.llvm.org/D117616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D45438: [CodeView] Enable debugging of captured variables within C++ lambdas

dblaikie added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:812-814 + // CodeView types with C++ mangling need a type identifier. + if (CGM.getCodeGenOpts().EmitCodeView) +return true; bwyma wrote: > dblaikie wrote: > > rnk wrote: > > > rnk wrot

[PATCH] D118109: [clang] Replace `std::vector` use in Syntax

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

[PATCH] D117626: [ADT] [NFC] Add StringRef::detectEOL

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Sounds good to me Comment at: llvm/unittests/ADT/StringRefTest.cpp:1114-1116 + for (const auto &Entry : Cases) { +EXPECT_EQ(StringRef("\n"), Entry.detectEOL()); + }

[PATCH] D117744: [Driver] Remove obsoleted -gz=zlib-gnu

dblaikie added a comment. > According to Debian Code Search, no project uses -gz=zlib-gnu (valgrind has a > configure to use -gz=zlib) Could you link to the queries you used to validate this? I'd be curious to take a look around. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D5980: unique_ptrify ownership of ModuleFiles in the ModuleManager

dblaikie added a comment. In D5980#3253378 , @dblaikie wrote: > Looks like @dexonsmith got this (putting the ownership in `Chain` & using > some iterator adapters to narrow the cleanup scope/fallout) in > rGa897f7cd40b12f693acbb04b81ef928244637aae >

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

dblaikie added inline comments. Comment at: clang/lib/CodeGen/Address.h:30 + // Int portion stores lower 3 bits of the log of the alignment. + llvm::PointerIntPair ElementType; nikic wrote: > dblaikie wrote: > > aeubanks wrote: > > > nikic wrote: > > > > Are

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

dblaikie added inline comments. Comment at: clang/lib/CodeGen/Address.h:30 + // Int portion stores lower 3 bits of the log of the alignment. + llvm::PointerIntPair ElementType; aeubanks wrote: > nikic wrote: > > Are we guaranteed 3 bits even on 32-bit archite

[PATCH] D5980: unique_ptrify ownership of ModuleFiles in the ModuleManager

dblaikie abandoned this revision. dblaikie added a subscriber: dexonsmith. dblaikie added a comment. Herald added a subscriber: yaxunl. Looks like @dexonsmith got this (putting the ownership in `Chain` & using some iterator adapters to narrow the cleanup scope/fallout) in a897f7cd40b12f693acbb04

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

dblaikie created this revision. dblaikie added reviewers: rnk, rjmccall, rsmith. dblaikie requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This matches GCC: https://godbolt.org/z/sM5q95PGY I realize this is an API break for clang+clang - so

[PATCH] D117321: [clang] Remap file path in __PRETTY_FUNCTION__

dblaikie added a comment. This seems like patching a hole, but maybe something more fundamental should be done if this remapping is meant to apply to all path rendering? (presumably there could be other bugs where these callbacks aren't applied - so is there somewhere lower level that this issu

[PATCH] D45438: [CodeView] Enable debugging of captured variables within C++ lambdas

dblaikie added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:812-814 + // CodeView types with C++ mangling need a type identifier. + if (CGM.getCodeGenOpts().EmitCodeView) +return true; rnk wrote: > rnk wrote: > > dblaikie wrote: > > > Just came

[PATCH] D45438: [CodeView] Enable debugging of captured variables within C++ lambdas

dblaikie added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:812-814 + // CodeView types with C++ mangling need a type identifier. + if (CGM.getCodeGenOpts().EmitCodeView) +return true; Just came across this code today - it's /probably/ a problem

[PATCH] D116313: [MSVC] Silence -Wnon-virtual-dtor on DIA APIs

dblaikie added a comment. In D116313#3222508 , @hans wrote: > In D116313#3218224 , @aganea wrote: > >> I was wondering, can we mandate `LLVM_ENABLE_WERROR=ON` for all bots at >> least? (or maybe enable it through

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

dblaikie added a subscriber: thakis. dblaikie added a comment. @thakis - I think you folks over in Chrome land implemented/use `-fdebug-prefix-map` (or do you use `-fdebug-compilation-dir`?) so might be interested in this change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113718/new

[PATCH] D115503: [DebugInfo][Clang] record the access flag for class/struct/union types.

dblaikie added a comment. In D115503#3199489 , @shchenz wrote: > In D115503#3199459 , @Esme wrote: > >> In D115503#3195171 , @dblaikie >> wrote: >> >>> Ah, cool - could y

[PATCH] D115503: [DebugInfo][Clang] record the access flag for class/struct/union types.

dblaikie added a comment. In D115503#3193978 , @Esme wrote: > In D115503#3192840 , @dblaikie > wrote: > >> Thanks for the data - looks good to me. Maybe include some of that data >> (summary of total binary size

[PATCH] D115503: [DebugInfo][Clang] record the access flag for class/struct/union types.

dblaikie added a comment. In D115503#3191156 , @Esme wrote: > In D115503#3188302 , @dblaikie > wrote: > >> Got any data on how much this (combined with the LLVM patch) increases debug >> info size of, say, a cla

[PATCH] D115503: [DebugInfo][Clang] record the access flag for class/struct/union types.

dblaikie accepted this revision. dblaikie added a comment. Got any data on how much this (combined with the LLVM patch) increases debug info size of, say, a clang self-host build? I assume not much, but wouldn't hurt to know. (does GCC produce this sort of debug info, or does it skip the access

[PATCH] D115503: [DebugInfo][Clang] record the access flag for class/struct/union types.

dblaikie added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-access.cpp:28 +class C { +public: shchenz wrote: > Will we generate a redundant flag if we define a private type in the class or > a public type in the struct/union? Maybe we could ad

[PATCH] D115503: [DebugInfo] emit DW_AT_accessibility attribute for class/struct/union types.

dblaikie added a comment. Generally sounds OK to me - this should be submitted as two separate patches. The LLVM part first, followed by the Clang part (since they can be separated they should be, and I guess the order doesn't matter too much (since the new IR that Clang's generating won't be i

[PATCH] D115379: ASTMatchers: Avoid using SmallVector::set_size()

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115379/new/ https://reviews.llvm.org/D115379

[PATCH] D115386: AST: Avoid using SmallVector::set_size() in UnresolvedSet

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good. Comment at: clang/lib/Sema/SemaLookup.cpp:623 - Decls.set_size(N); + Decls.truncate(N); dexonsmith wrote: > Two things to confirm here.

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

dblaikie added a comment. In D113743#3174073 , @krisb wrote: > In D113743#3173981 , @JDevlieghere > wrote: > >> Hey Kristina, this broke TestSetData.py on GreenDragon: >> https://green.lab.llvm.org/green/view/LL

[PATCH] D115094: Fix -Wdeclaration-after-statement doesn't work when used with -std=c99

dblaikie added a comment. This'll at least need a test case added - though the specifics of how the warning should work I'll leave up to @aaron.ballman - unless he wants some second opinions. (I don't immediately have a strong feeling either way regarding the current or proposed behavior) Rep

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

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Guessing Adrian probably meant to sign off on this, judging by his phrasing but maybe forgot to hit the "accept" button. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77598: Integral template argument suffix and cast printing

dblaikie added inline comments. Comment at: clang/lib/AST/DeclPrinter.cpp:1101 Out << ", "; -Args[I].print(Policy, Out); +if (TemplOverloaded || !Params) + Args[I].print(Policy, Out, /*IncludeType*/ true); rsmith wrote: > dblaikie wrote: > > d

<    2   3   4   5   6   7   8   9   10   11   >