[PATCH] D143967: [DebugInfo][BPF] Add 'btf:type_tag' annotation in DWARF

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Fair enough - all seems a bit unfortunate to be pushing these attributes through to places they don't technically apply to (both complicates the implementation, and might be confusing to u

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

dblaikie added subscribers: sammccall, dblaikie. dblaikie added a comment. @sammccall @rsmith - figure if this does impact us, we'll use the flag to opt-out in the short term while we figure out longer-term solution, yeah? Is there any pre-emptive testing you think is worth doing? Repository:

[PATCH] D158296: [clang] Diagnose overly complex Record in __builtin_dump_struct

dblaikie added a comment. @rsmith ping on this thread - wondering what you'd prefer the direction be here Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158296/new/ https://reviews.llvm.org/D158296 ___ cf

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

dblaikie added a comment. In D150226#4650244 , @carlosgalvezp wrote: > @shafik @aaron.ballman @dblaikie > > Hello! Just wanted to check if there's any blockers for merging this patch? > We are now on Clang 18, i.e. 2 releases after the warning was intro

[PATCH] D158730: [DebugMetadata][DwarfDebug] Don't retain local types with -fno-eliminate-unused-debug-types

dblaikie accepted this revision. dblaikie added a comment. Nice and easy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158730/new/ https://reviews.llvm.org/D158730 ___ cfe-commits mailing list cfe-commit

[PATCH] D157615: [ExtendLifetimes][1/4] Add "disable-post-ra" function attribute to disable the post-regalloc scheduler

dblaikie added a comment. In D157615#4647235 , @StephenTozer wrote: > In D157615#4646848 , @dblaikie > wrote: > >> Might be worth rewording the commit, or splitting it - I'd say the >> introduction of `optdebu

[PATCH] D157615: [ExtendLifetimes][1/4] Add "disable-post-ra" function attribute to disable the post-regalloc scheduler

dblaikie added a comment. Might be worth rewording the commit, or splitting it - I'd say the introduction of `optdebug` should be the noteworthier part of this patch (or whichever patch introduces it) - so either "this patch adds optdebug, and a first/exemplar use of it in postra scheduler" or

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

dblaikie added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:14524 + VD->hasAttr() && + (!VD->getType().isPODType(getASTContext()) || + !VD->getType().isConstQualified())) { aeubanks wrote: > rnk wrote: > > I don't think `isPODType` rea

[PATCH] D158296: [clang] Diagnose overly complex Record in __builtin_dump_struct

dblaikie added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:732-733 +int RDKind = RD->isClass() ? 0 : (RD->isStruct() ? 1 : 2); +S.Diag(PtrArg->getBeginLoc(), diag::err_builtin_dump_struct_too_complex) +<< RDKind << RD->getName(); +return ExprE

[PATCH] D158496: [WIP][clang][modules] module build daemon initial commit

dblaikie added a comment. (probably worth linking to the RFC/etc (https://discourse.llvm.org/t/rfc-modules-build-daemon-build-system-agnostic-support-for-explicitly-built-modules/71524/47) from the patch description/commit message) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D151575: [clang][diagnostics] Always show include stacks on top-level diagnostics

dblaikie added a comment. > Maybe we should consider adding a flag to set diagnostic verbosity level? > e.g., -fdiagnostic-verbosity=terse|default|verbose where terse never prints > notes or include stacks, default is what we do today, and verbose always > prints include stacks? My guess would

[PATCH] D154784: [clang] Fix crash caused by PseudoObjectExprBitfields::NumSubExprs overflow

dblaikie added inline comments. Comment at: clang/include/clang/AST/Stmt.h:596-597 -// These don't need to be particularly wide, because they're -// strictly limited by the forms of expressions we permit. -unsigned NumSubExprs : 8; -unsigned ResultIndex : 32 - 8

[PATCH] D154784: [clang] Fix crash caused by PseudoObjectExprBitfields::NumSubExprs overflow

dblaikie added inline comments. Comment at: clang/include/clang/AST/Stmt.h:596-597 -// These don't need to be particularly wide, because they're -// strictly limited by the forms of expressions we permit. -unsigned NumSubExprs : 8; -unsigned ResultIndex : 32 - 8

[PATCH] D158301: Add back overriding-t-options for -m-version-min diagnostic

dblaikie added inline comments. Comment at: clang/test/Driver/darwin-version.c:217 // RUN: FileCheck --check-prefix=CHECK-VERSION-TNO-OSV1 %s -// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with '-target x86_64-apple-macos10.11.2' +// CHECK-VERSION-TN

[PATCH] D157762: [WIP] Implement [[msvc::no_unique_address]]

dblaikie added inline comments. Comment at: clang/lib/AST/Decl.cpp:4523-4524 bool FieldDecl::isPotentiallyOverlapping() const { - return hasAttr() && getType()->getAsCXXRecordDecl(); + return (hasAttr() || + hasAttr()) && + getType()->getAsCXXRecordDecl(); ---

[PATCH] D158296: [NFC][Clang] Add assertion to check the value of NumSubExprs/ResultIndex does not overflow

dblaikie added a comment. Hopefully my clarification on https://reviews.llvm.org/D154784#inline-1531176 makes it clear that assertions aren't necessarily the right tool here - as this path is reachable with actual code & I don't think we should allow that code to reach UB in the compiler in a n

[PATCH] D154784: [clang] Fix crash caused by PseudoObjectExprBitfields::NumSubExprs overflow

dblaikie added inline comments. Comment at: clang/include/clang/AST/Stmt.h:596-597 -// These don't need to be particularly wide, because they're -// strictly limited by the forms of expressions we permit. -unsigned NumSubExprs : 8; -unsigned ResultIndex : 32 - 8

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

dblaikie added a comment. In D158137#4599481 , @MaskRay wrote: > In D158137#4599461 , @dblaikie > wrote: > >> In D158137#4597491 , @dexonsmith >> wrote: >> >>> In D15813

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

dblaikie added a comment. In D158137#4597491 , @dexonsmith wrote: > In D158137#4597009 , @MaskRay wrote: > >> In D158137#4596948 , @dexonsmith >> wrote: >> >>> Can you e

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

dblaikie added a comment. In D156726#4594005 , @yonghong-song wrote: > In D156726#4593671 , @dblaikie > wrote: > >> Fixed in 19c216b5ab7c34c297b4c3c4da64b49b3c830a3a > > Hi, @dblaikie I cannot find the above com

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

dblaikie added a comment. Fixed in 19c216b5ab7c34c297b4c3c4da64b49b3c830a3a Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156726/new/ https://reviews.llvm.org/D156726 ___ cfe-commits mailing list cfe-com

[PATCH] D158006: [Clang][WIP]Experimental implementation of data member packs in dependent context.

dblaikie added inline comments. Comment at: clang/lib/Sema/SemaExprMember.cpp:519-521 + if (const ElaboratedType *ET = dyn_cast(BaseType)) { +if (const TemplateSpecializationType *TST = +dyn_cast(ET->getNamedType())) { I think you can skip the `c

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

dblaikie added a comment. In D156726#4591543 , @eddyz87 wrote: > As an additional data point, the same example but w/o section specification > compiles fine: > > const int with_init = 1; > const int no_init; > > And puts both globals to the same sect

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Sounds OK to me - thanks! bit more detail in the actual patch description/commit message (from some of the comments in this review, perhaps) would be handy. CHANGES SINCE LAST ACTION h

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG19f2b68095fe: Make globals with mutable members non-constant, even in custom sections (authored by dblaikie). Repository: rG LLVM Github Monorepo

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

dblaikie added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:14254 int SectionFlags = ASTContext::PSF_Read; -if (var->getType().isConstQualified()) { - if (HasConstInit) rnk wrote: > efriedma wrote: > > dblaikie wrote: > > > efriedma wrot

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

dblaikie updated this revision to Diff 550097. dblaikie added a comment. Diagnose specific reasons Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156726/new/ https://reviews.llvm.org/D156726 Files: clang/include/clang/AST/Type.h clang/include/c

[PATCH] D157762: [WIP] Implement [[msvc::no_unique_address]]

dblaikie added inline comments. Comment at: clang/lib/AST/Decl.cpp:4510-4512 +for (const FieldDecl *Field : CXXRD->fields()) + if (Field->getType()->getAsCXXRecordDecl()) +return false; maybe `llvm::any_of`? Not especially shorter, but maybe make

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

dblaikie added a comment. @rnk & I figured that the user probably isn't relying on this much - if they cared about a particular variable, they could use an attribute instead of a pragma. The pragma just being about "put all my const things over here and all my non-const things over there" - bug

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

dblaikie added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:14254 int SectionFlags = ASTContext::PSF_Read; -if (var->getType().isConstQualified()) { - if (HasConstInit) efriedma wrote: > dblaikie wrote: > > rnk wrote: > > > rsmith wrote:

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

dblaikie updated this revision to Diff 549206. dblaikie added a comment. Add a warning for MSVC pragma inconsistency Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156726/new/ https://reviews.llvm.org/D156726 Files: clang/include/clang/AST/Type.h

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

dblaikie added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:14254 int SectionFlags = ASTContext::PSF_Read; -if (var->getType().isConstQualified()) { - if (HasConstInit) rnk wrote: > rsmith wrote: > > efriedma wrote: > > > rnk wrote: > >

[PATCH] D154784: [clang] Fix crash caused by PseudoObjectExprBitfields::NumSubExprs overflow

dblaikie added inline comments. Comment at: clang/include/clang/AST/Stmt.h:596-597 -// These don't need to be particularly wide, because they're -// strictly limited by the forms of expressions we permit. -unsigned NumSubExprs : 8; -unsigned ResultIndex : 32 - 8

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

dblaikie created this revision. Herald added subscribers: kerbowa, jvesely. Herald added a project: All. dblaikie requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Turned out we were making overly simple assumptions about which sections (& s

[PATCH] D143967: [DebugInfo][BPF] Add 'btf:type_tag' annotation in DWARF

dblaikie added a subscriber: aprantl. dblaikie added a comment. In D143967#4441333 , @eddyz87 wrote: > In D143967#4438815 , @dblaikie > wrote: > >> I haven't looked closely at this, but from a vague/quick reading

[PATCH] D154784: [clang] Fix crash caused by PseudoObjectExprBitfields::NumSubExprs overflow

dblaikie added inline comments. Comment at: clang/include/clang/AST/Stmt.h:596-597 -// These don't need to be particularly wide, because they're -// strictly limited by the forms of expressions we permit. -unsigned NumSubExprs : 8; -unsigned ResultIndex : 32 - 8

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

dblaikie added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3279 + assert(NumArgumentsInExpansion && "should not see unknown template argument here"); + for (unsigned Arg = 0; Arg < NumArgumentsInExpansion; ++Arg) { +// Generate a new

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

dblaikie added a comment. From the other review: >> Could/should we do the lookup on the CU filename before it goes into the DI >> metadata, and store that FileID somewhere for later use? > > There's a note in issue 63955 to the effect that I can't find an API to turn > a filename into a FileID

[PATCH] D156546: [Clang][WIP]Experimental implementation of packed data members declarations

dblaikie added a comment. (just a side note about the title of this patch: I got "packed data members" confused and thought this was referring to struct packing `__attribute__((packed))` - so perhaps something more like "data member packs" would be a more clear term here?) C

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. FWIW this sounds good to me (though given how wide the patch is, might be worth waiting a few days to a week in case anyone else has thoughts). I only looked at a sample of the test change

[PATCH] D154784: [clang] Fix crash caused by PseudoObjectExprBitfields::NumSubExprs overflow

dblaikie added inline comments. Comment at: clang/include/clang/AST/Stmt.h:596-597 -// These don't need to be particularly wide, because they're -// strictly limited by the forms of expressions we permit. -unsigned NumSubExprs : 8; -unsigned ResultIndex : 32 - 8

[PATCH] D154784: [clang] Fix crash caused by PseudoObjectExprBitfields::NumSubExprs overflow

dblaikie added inline comments. Comment at: clang/include/clang/AST/Stmt.h:596-597 -// These don't need to be particularly wide, because they're -// strictly limited by the forms of expressions we permit. -unsigned NumSubExprs : 8; -unsigned ResultIndex : 32 - 8

[PATCH] D154366: [clang][ExprConstant] Print template arguments when describing stack frame

dblaikie added a comment. @hazohelet can you commit this yourself, or do you need someone to commit it on your behalf? (& if so, what name/email address would you like to be credited) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154366/new/ https://reviews.llvm.org/D154366 __

[PATCH] D156143: Add Adrian and David as owners for debug info

dblaikie added a comment. Sounds good, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156143/new/ https://reviews.llvm.org/D156143 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

[PATCH] D155991: [DWARF] Make sure file entry for artificial functions has an MD5 checksum

dblaikie added a comment. Any memory usage measurements to check this doesn't have a significant adverse impact by copying all the strings? (or performance by having to do string rather than pointer equality comparisons?) Could/should we do the lookup on the CU filename before it goes into the

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

dblaikie added a comment. In D153536#4521156 , @hubert.reinterpretcast wrote: > In D153536#4512897 , @dblaikie > wrote: > >> but at least at a first blush I can't reproduce the failures shown... > > @dblaikie, y

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

dblaikie added a comment. > I think getting new system headers is harder for projects to deal with, so I > think I'm now in agreement with you that we should enable the warning as an > error in system headers. Awesome - glad we're on the same page :) CHANGES SINCE LAST ACTION https://review

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

dblaikie added a comment. In D150226#4515843 , @aaron.ballman wrote: > In D150226#4515783 , @dblaikie > wrote: > >> In D150226#4498024 , >> @aaron.ballman wrote: >> >>>

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

dblaikie added a comment. In D150226#4498024 , @aaron.ballman wrote: > In D150226#4461846 , @efriedma > wrote: > >> The fundamental problem here is the interaction with SFINAE; if we don't >> diagnose this as a

[PATCH] D134544: [clang-cl] Implement /ZH: flag

dblaikie added inline comments. Comment at: clang/include/clang/Driver/Options.td:3220 Values<"simple,mangled">, Flags<[CC1Option, NoDriverOption]>; +def gsrc_hash_EQ : Joined<["-"], "gsrc-hash=">, + Group, Flags<[CC1Option, NoDriverOption]>, thakis wrote

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

dblaikie added a comment. In D153536#4490918 , @cor3ntin wrote: > @dblaikie Would you be willing to look at the debugger side of things in a > subsequent patch? I'm not familiar with debug symbol code gen so I'm not sure > I'd be able to improve thing t

[PATCH] D154366: [clang][ExprConstant] Print template arguments when describing stack frame

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Sounds good to me CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154366/new/ https://reviews.llvm.org/D154366 ___ cfe-commits mailing l

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

dblaikie added a comment. In D153536#4475308 , @cor3ntin wrote: > In D153536#4475240 , @dblaikie > wrote: > >> In D153536#4475199 , @cor3ntin >> wrote: >> >>> In D153536

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

dblaikie added a comment. In D153536#4475199 , @cor3ntin wrote: > In D153536#4474733 , @dblaikie > wrote: > >> Maybe try testing with more different values, to demonstrate which entities >> the debugger is findi

[PATCH] D154366: [clang][ExprConstant] Print template arguments when describing stack frame

dblaikie added a comment. Do we not already have some function name printing helper we could be reusing that might have some smarts about not printing deduced template arguments, using preferred names, whatever else? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

dblaikie added a comment. Maybe try testing with more different values, to demonstrate which entities the debugger is finding? int _ = 1; int _ = 2; { int _ = 3; int _ = 7; } Or something like that. But, honestly, if the point is that these variables should be unnameable - perh

[PATCH] D152017: [DebugInfo] Add flag to only emit referenced member functions

dblaikie added a comment. In D152017#4462263 , @probinson wrote: >> we might emit member function declarations for call sites in DWARF, for >> instance > > So are you now leaning more toward wanting to emit the "used" declarations as > well? I'm sure yo

[PATCH] D153898: [DebugInfo] Enable debug info emission for extern variables in C++

dblaikie accepted this revision. dblaikie added a comment. In D153898#4457263 , @chiyuze wrote: > Regarding cost, only BPF target triggers this debug info generation for > extern variables. Ah, OK - if that's what you need for BPF and it doesn't affect

[PATCH] D153898: [DebugInfo] Enable debug info emission for extern variables in C++

dblaikie added a comment. Got some details on how much this costs? (eg: `bloaty` comparison for a clang bootstrap with/without this patch applied?) Could you provide a quick summary of why this debug info is required? The definition should be provided in whatever translation unit defines the var

[PATCH] D152017: [DebugInfo] Add flag to only emit referenced member functions

dblaikie added a comment. Basing this on "defined" or "undefined" member functions isn't /perfectly/ correct (though might be correct enough for this flag - though I do prefer something about "canonicality" of types, but perhaps it's too inside-baseball for the average user - though these debug

[PATCH] D143967: [DebugInfo][BPF] Add 'btf:type_tag' annotation in DWARF

dblaikie added a comment. I haven't looked closely at this, but from a vague/quick reading, it sounds like this is creating annotations on certain type DINodes, but then moving them around to different types? It'd be nice if we could avoid that/create them in the right place in the first place.

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

dblaikie added a comment. Any particular use for this debug info in DWARF consumers? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153362/new/ https://reviews.llvm.org/D153362 ___ cfe-commits mailing lis

[PATCH] D151575: [clang][diagnostics] Always show include stacks on errors

dblaikie added a comment. Judging by @aaron.ballman's comment on the bug, I can see the logic here. Though perhaps this notion generalizes to all non-note types, not just errors (like perhaps we should print the include stack for each warning, too?) Repository: rG LLVM Github Monorepo CHANG

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

dblaikie added a comment. > My concern with `ShowInSystemHeaders` is that this seems like a bad user > experience. I guess you're comparing this user experience to the one when this feature was a normal warning/without `ShowInSystemHeaders` - I'm comparing this situation to the future where th

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

dblaikie added a comment. In D150226#4417539 , @aaron.ballman wrote: > In D150226#4408980 , @dblaikie > wrote: > >> In D150226#4408563 , @erichkeane >> wrote: >> >>> In

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

dblaikie added a comment. Are there any known (or vague/unknown) limitations on the implementation with respect to the actual output/on-disk representation? (like, would doing some kind of size analysis of the output when using this patch/feature lead to incorrect conclusions about the cost of

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

dblaikie added a comment. Really appreciate you providing this prototype. > This solves one of the current weaknesses in DWARF debugging, which is the > inability to set breakpoints or step onto inlined callsites. However, there > are other proposals (such as Location View Numbering) that could

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

dblaikie added a comment. When we migrated to github we gained higher fidelity for attributing authors (we can now commit on someone's behalf but have the commit author properly recorded as the real author - we couldn't do that back on Subversion) - but I think it's meant that fail-mail from bo

[PATCH] D152447: [Clang] Remove -no-opaque-pointers cc1 flag

dblaikie added subscribers: t.p.northover, dblaikie. dblaikie added a comment. (I hope this doesn't come off as condescending/patronizing, I mean it quite genuinely) A deep thanks to everyone who worked on this, especially @aeubanks and @nikic, and @t.p.northover - it's been a long project and

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

dblaikie added a comment. In D150226#4408563 , @erichkeane wrote: > In D150226#4408381 , @aaron.ballman > wrote: > >> In D150226#4404808 , @rupprecht >> wrote: >> >>> I

[PATCH] D124221: Reimplement `__builtin_dump_struct` in Sema.

dblaikie added a comment. https://github.com/llvm/llvm-project/issues/63169 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124221/new/ https://reviews.llvm.org/D124221 ___ cfe-commits mailing list cfe-com

[PATCH] D152017: [DebugInfo] Add flag to only emit referenced member functions

dblaikie added a comment. In D152017#4400735 , @probinson wrote: > Oh, `-fstandalone-debug` should override this? In the Sony implementation, it > does. (Sorry for not remembering that sooner.) I'd think of this as orthogonal to that, potentially. by an

[PATCH] D152017: [DebugInfo] Add flag to only emit referenced member functions

dblaikie added a comment. In D152017#4399989 , @probinson wrote: > My debugger guy says "this shouldn't be a problem." > > Given that, my request is that `-gincomplete-types` should be default-true > for `DebuggerTuning == SCE` if you want to commit this

[PATCH] D152017: [DebugInfo] Add flag to only emit referenced member functions

dblaikie added a comment. oh, side note: We don't emit DWARF descriptions of called non-member functions, generally (well, more often these days with optimized debug info call_site info we do need to emit function declarations for called functions) - so by analogy I'd have thought doing similar

[PATCH] D152017: [DebugInfo] Add flag to only emit referenced member functions

dblaikie added a comment. In D152017#4396906 , @probinson wrote: > I experimented with this. Looks like it emits info only for //defined// > methods, and not //used// methods. That is, if you change the test to say > `void t1::f1() { f2(); }` > you get D

[PATCH] D152017: [DebugInfo] Add flag to only emit referenced member functions

dblaikie updated this revision to Diff 527920. dblaikie added a comment. Updating commit message with extra details Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152017/new/ https://reviews.llvm.org/D152017 Files: clang/include/clang/Basic/CodeG

[PATCH] D152017: [DebugInfo] Add flag to only emit referenced member functions

dblaikie created this revision. dblaikie added a reviewer: probinson. Herald added a project: All. dblaikie requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Complete C++ type information can be quite expensive - and there's limited v

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

dblaikie added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:5562 + if (IsDependent) +goto Return; + erichkeane wrote: > Oh, please don't do this. perhaps another way to do this if you want to avoid repeating the return expression would be a s

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

dblaikie added inline comments. Comment at: clang/include/clang/AST/Stmt.h:796-802 +/// If this expression is not value-dependent, this stores the value. +unsigned Value : 8; /// The number of arguments to this type trait. According to [implimits] /// 8 bits w

[PATCH] D151162: Add -Wpacked-non-pod to -Wall

dblaikie added inline comments. Comment at: clang/test/CodeGenCXX/warn-padded-packed.cpp:5 +// RUN: %clang_cc1 -triple=x86_64-none-none -Wpacked-non-pod -verify=top %s -emit-llvm-only +// RUN: %clang_cc1 -triple=x86_64-none-none -Wpacked-non-pod -fclang-abi-compat=15 %s -emit-l

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

dblaikie added a comment. yeah, doesn't look like it found any bugs there (the `arrow` one seems the most non-trivwial, but I'm guessing the code as-is is correct), so seems a bit questionable as an addition But @aaron.ballman if you figure this falls more under the "minor tweak to existing di

[PATCH] D150411: [NFC][Clang][Coverity] Fix Static Code Analysis Concerns with copy without assign

dblaikie added a comment. (& I realize I'm a bit late to the party, but: > In several cases it's not completely obvious to me whether a copy assignment > operator should or can be defined. But perhaps this doesn't need to be > addressed right now: we seem to compile with -Wextra, which contains

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

dblaikie added a comment. In D143984#4358115 , @dzhidzhoev wrote: > In D143984#4357517 , @dblaikie > wrote: > >> Looks like this series got approved - is it waiting on any particular >> feedback, or do you need

[PATCH] D151162: Add -Wpacked-non-pod to -Wall

dblaikie added a comment. Yeah, looks consistent with GCC ( https://godbolt.org/z/EGc3Ec9YT ), as you say - so I'm OK with it. But wouldn't mind a second opinion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151162/new/ https://reviews.llvm.org/D

[PATCH] D148851: Disable llvm-symbolizer on some of the driver tests that are timing out

dblaikie added a comment. In D148851#4357360 , @arsenm wrote: > I also think we need to revert or disable > https://github.com/llvm/llvm-project/commit/cead4eceb01b935fae07bf4a7e91911b344d2fec > > The symbolizer is unusably slow with it I believe that i

[PATCH] D148851: Disable llvm-symbolizer on some of the driver tests that are timing out

dblaikie added a comment. In D148851#4285705 , @dblaikie wrote: >> I think we probably should add LLVM_DISABLE_SYMBOLIZATION=1 to the lit >> level, not in individual tests. > > Though I'm not sure how to do that in a way that it doesn't apply to test >

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

dblaikie added a comment. Looks like this series got approved - is it waiting on any particular feedback, or do you need someone to commit it for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143984/new/ https://reviews.llvm.org/D143984

[PATCH] D149677: [clang][TypePrinter] Add option to skip over elaborated types

dblaikie added a comment. In D149677#4356863 , @aaron.ballman wrote: > In D149677#4350337 , @li.zhe.hua > wrote: > >> In D149677#4349523 , >> @aaron.ballman wrote: >> >

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

dblaikie added a comment. In D147844#4329497 , @aaron.ballman wrote: > In general, I think this is incremental progress on the diagnostic behavior. > However, it's clear that there is room for interpretation on what is or is > not a false positive diag

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. In D149193#4328553 , @MaskRay wrote: > In D149193#4328452 , @dblaikie > wrote: > >>> I agree that for mo

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

dblaikie added a comment. > I agree that for most(all?) split DWARF users will not see any difference > since they always use -c -o and don't combine compilation and linking in one > command. Given that, I'm not sure that this is worth implementing, but if it suits you I guess.

[PATCH] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

dblaikie added a comment. Got a link to a design discussion motivating this change? I'd have thought it made sense to put modulemaps in subdirectories - since they cover the whole directory, putting them in the root of an include path would be problematic if there are multiple distinct projects

[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

dblaikie added inline comments. Comment at: clang/docs/ReleaseNotes.rst:109-113 +Warnings + +- Address a false positive in ``-Wpacked`` when applied to a non-pod type using + Clang ABI >= 15 (fixes `#62353`_, + fallout

[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

dblaikie added inline comments. Comment at: clang/docs/ReleaseNotes.rst:109-113 +Warnings + +- Address a false positive in ``-Wpacked`` when applied to a non-pod type using + Clang ABI >= 15 (fixes `#62353`_, + fallout

[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. dblaikie marked an inline comment as done. Closed by commit rGa8b0c6fa28ac: Remove -Wpacked false positive for non-pod types where the layout isn't… (authored by dblaik

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

dblaikie added a comment. In D141451#4312256 , @aaron.ballman wrote: > In D141451#4311199 , @dblaikie > wrote: > >>> probably too much, but then I wonder "how do we make Fortify work?". >> >> (I'm still sort of

[PATCH] D148851: Disable llvm-symbolizer on some of the driver tests that are timing out

dblaikie added a comment. In D148851#4311266 , @ahatanak wrote: > Disable llvm-symbolizer when running lit tests. This seems problematic though - when lit tests fail it's quite helpful to get a symbolized stack trace. Repository: rG LLVM Github Mono

[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

dblaikie marked an inline comment as done. dblaikie added a comment. In D149182#4312202 , @aaron.ballman wrote: > Precommit CI found a valid issue (at least on Debian, the Windows failure > appears to be unrelated but it's really hard to tell from that

[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

dblaikie updated this revision to Diff 519247. dblaikie added a comment. Describe condition in comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149182/new/ https://reviews.llvm.org/D149182 Files: clang/docs/ReleaseNotes.rst clang/lib/AST/

[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

dblaikie updated this revision to Diff 519245. dblaikie added a comment. Release note Restrict change to Clang ABI 16 and above (& test that condition) Fix AIX test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149182/new/ https://reviews.llvm.org/

  1   2   3   4   5   6   7   8   9   10   >