[clang] [CodeGen] Don't include CGDebugInfo.h in CodeGenFunction.h (NFC) (PR #134100)

2025-04-02 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. https://github.com/llvm/llvm-project/pull/134100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)

2025-03-31 Thread David Blaikie via cfe-commits
dwblaikie wrote: > > Removing the vtable global variable and moving the "location info" into the > > static within the class, will work for the SCE debugger. > > I was thinking about this last night and wondering if the vtable will appear > as a class member even if the class is local to a fun

[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)

2025-03-25 Thread David Blaikie via cfe-commits
dwblaikie wrote: > FYI: There is already VTable support in our lldb::SBValue class and it is > part of the public API in LLDB and doesn't require any of this: > ... > Doesn't require any debug info. Does this/can this be used to determine the type of an object that points to that vtable, thoug

[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)

2025-03-24 Thread David Blaikie via cfe-commits
dwblaikie wrote: > The link from the class to the specific vtable even seems mildly incorrect, > in that during object construction the vtable will go through several > different values, not just one. Not sure I follow this - the object is only of the type, in some sense, when it is pointing

[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)

2025-03-17 Thread David Blaikie via cfe-commits
dwblaikie wrote: > It would be better if we could go from the vtable pointer straight to the > right type DIE, but I don't think we can do that without inventing a new sort > of an accelerator table (which I guess we don't have an appetite for). yeah, data address lookup (as opposed to code ad

[clang] [llvm] Add support for template as type parameter (PR #127654)

2025-03-12 Thread David Blaikie via cfe-commits
dwblaikie wrote: > Could you please take a look at the attached DWARF and let me know your > concerns with the implementation? Sure - I see the effect, but I still think it's probably not worthwhile due to the limited capacity, inconsistency (due to that limited expressiveness) between direct

[clang] [llvm] Add support for template as type parameter (PR #127654)

2025-03-11 Thread David Blaikie via cfe-commits
dwblaikie wrote: > Are you suggesting that for the implementation to be considered as complete, > both v1 and v2 should have the same type information? I.e "v1" type should > point to 0x48 instead of 0x6d? As per my understanding based on the DWARF > output below, the type for "trait::type"(0x

[clang] [Docs] Explain how to propose an extension in Clang (PR #130803)

2025-03-11 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. Sounds good to me - thanks for writing it up! (could wait for other folks to chime in too, but seems pretty harmless to launch/iterate/etc) https://github.com/llvm/llvm-project/pull/130803 ___ c

[clang] [llvm] Add support for template as type parameter (PR #127654)

2025-03-11 Thread David Blaikie via cfe-commits
dwblaikie wrote: > I am not sure that I understand your concern completely. Consider the > following DWARF output based on my implementation. How would you say "v1" > should be represented ideally? > ``` > 0x0042: DW_TAG_structure_type > DW_AT_calling_convention (DW_CC_pass_by_value) > DW_A

[clang] [llvm] Add support for template as type parameter (PR #127654)

2025-03-07 Thread David Blaikie via cfe-commits
dwblaikie wrote: > > Mostly I worry it won't be terribly complete, because it can't work through > > situations, like this: > > ``` > > template > > struct trait { > > using type = T; > > }; > > template > > struct other { > > trait::type v1; > > T v2; > > }; > > ``` > > In this case, v2

[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)

2025-03-07 Thread David Blaikie via cfe-commits
dwblaikie wrote: I wouldn't mind a few more details here on the motivation. > This new symbol will allow a debugger to easily associate classes with the > physical location of their VTables using only the DWARF information. What sort of features are you picturing building with this? The DWAR

[clang] [llvm] Add support for template as type parameter (PR #127654)

2025-02-21 Thread David Blaikie via cfe-commits
dwblaikie wrote: > For template classes with type parameters, the information of which member > variables are of a "templated type" is lost. The debugger cannot > differentiate between "templated" types and "hardcoded" types. This is > because both types have the same debug information, so the

[clang] [clang-tools-extra] [flang] [lldb] [llvm] [mlir] [polly] Add static to command line option (cl::opt) (PR #126243)

2025-02-07 Thread David Blaikie via cfe-commits
dwblaikie wrote: > > > I don't quite follow the motivation for this, can you expand on this in > > > the description please? Right now this seems unnecessary for the changes > > > I can see in MLIR for example. > > > > > > The linked bug seems to explain it, I think? It seems to be the usual

[clang] [clang-tools-extra] [flang] [lldb] [llvm] [mlir] [polly] Add static to command line option (cl::opt) (PR #126243)

2025-02-07 Thread David Blaikie via cfe-commits
dwblaikie wrote: > I don't quite follow the motivation for this, can you expand on this in the > description please? Right now this seems unnecessary for the changes I can > see in MLIR for example. The linked bug seems to explain it, I think? It seems to be the usual "if something isn't/does

[clang] [StrTable] Fix modules build and clean up stale files (PR #125979)

2025-02-06 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. https://github.com/llvm/llvm-project/pull/125979 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Force AttributedStmtClass to not be scope parents (PR #125370)

2025-02-03 Thread David Blaikie via cfe-commits
dwblaikie wrote: Could you add a test case - check in clang/test to see if other tests for the diagnostic text in the original bug, and add a test case for that nearby (maybe the same file the diagnostic is already tested in)? https://github.com/llvm/llvm-project/pull/125370 __

[clang] [llvm] [WIP][clang][DebugInfo] Add new DW_AT_APPLE_enum_kind to encode enum_extensibility (PR #124752)

2025-02-03 Thread David Blaikie via cfe-commits
dwblaikie wrote: Is my thing about the ctor understanding correct, and it's that the ctor exists theoretically/abstractly, but not in the AST or in the generated IR/DWARF? Could it be added/would that be sufficient? But, yeah, probably fine to just add the attribute, since that's the real fea

[clang] [clang-cl]: generate debug info when `novtable` is specified (PR #124643)

2025-01-28 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie closed https://github.com/llvm/llvm-project/pull/124643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-cl]: generate debug info when `novtable` is specified (PR #124643)

2025-01-28 Thread David Blaikie via cfe-commits
dwblaikie wrote: Do you have commit access, or need someone to merge this on your behalf? https://github.com/llvm/llvm-project/pull/124643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[clang] [llvm] [WIP][clang][DebugInfo] Add new DW_AT_APPLE_enum_kind to encode enum_extensibility (PR #124752)

2025-01-28 Thread David Blaikie via cfe-commits
dwblaikie wrote: (hmm, can't use the foundation library on godbolt/compiler explorer) Can you show a small example of the code you're interested in and the DWARF it produces? The documentation I can find seems to suggest that the extensible enums have ctors that take the underlying integer type

[clang] [clang-cl]: generate debug info when `novtable` is specified (PR #124643)

2025-01-27 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. Looks great - thanks! https://github.com/llvm/llvm-project/pull/124643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [clang][DebugInfo] Emit DW_AT_object_pointer on function declarations with explicit `this` (PR #122928)

2025-01-23 Thread David Blaikie via cfe-commits
dwblaikie wrote: yeah, I was going to say - thought we carved out part (the debug info part, maybe some other things - ORC?) of the C API as unstable, so I'm OK continuing with that (lack of) guarantee... https://github.com/llvm/llvm-project/pull/122928

[clang] [llvm] Generate `DW_AT_RUST_short_backtrace` attributes for `DISubprogram` nodes (PR #123683)

2025-01-21 Thread David Blaikie via cfe-commits
dwblaikie wrote: > > For instance, the StartFrame/EndFrame feature, if I'm understanding it > > correctly, seems like maybe it could be skipped if we're emitting DWARF (& > > instead the skip attribute could be placed on all the calls between > > StartFrame and EndFrame?). > > does [#123683

[clang] [llvm] [StrTable] Switch diag group names to `llvm::StringTable` (PR #123302)

2025-01-17 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. Generally looks fine to me - given the central nature of the data structure, wouldn't mind a second/more authoritative set of eyes if someone has time. You mention the performance tradeoff of pascal strings v null terminated ones doesn't

[clang] [llvm] [StrTable] Switch diag group names to `llvm::StringTable` (PR #123302)

2025-01-17 Thread David Blaikie via cfe-commits
@@ -51,6 +53,14 @@ class StringTable { constexpr Offset() = default; constexpr Offset(unsigned Value) : Value(Value) {} +constexpr bool operator==(const Offset &RHS) const { dwblaikie wrote: Minor convention, but prefer non-member (can still be de

[clang] [llvm] [StrTable] Switch diag group names to `llvm::StringTable` (PR #123302)

2025-01-17 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie edited https://github.com/llvm/llvm-project/pull/123302 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [clang][DebugInfo] Emit DW_AT_object_pointer on function declarations with explicit `this` (PR #122928)

2025-01-17 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. Looks OK to me. https://github.com/llvm/llvm-project/pull/122928 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 504dd57 - DebugInfo: Avoid emitting null members for nodebug nested typedefs

2025-01-15 Thread David Blaikie via cfe-commits
Author: David Blaikie Date: 2025-01-15T23:20:40Z New Revision: 504dd577675e8c85cdc8525990a7c8b517a38a89 URL: https://github.com/llvm/llvm-project/commit/504dd577675e8c85cdc8525990a7c8b517a38a89 DIFF: https://github.com/llvm/llvm-project/commit/504dd577675e8c85cdc8525990a7c8b517a38a89.diff LOG:

[clang] [clang-tools-extra] Remove `StringLiteral` in favor of `StringRef` (PR #122366)

2025-01-13 Thread David Blaikie via cfe-commits
dwblaikie wrote: > > Just that the last bit of performance > > Maybe we're talking about different situations; I may have missed some > context in the discussion of this PR. I didn't see this as being about the > last bit of performance. It sounded like this was going to knowingly > introduce

[clang] [clang-tools-extra] Remove `StringLiteral` in favor of `StringRef` (PR #122366)

2025-01-13 Thread David Blaikie via cfe-commits
dwblaikie wrote: > > > Anyway, my main point was that there are people who care about > > > performance on Windows, so please don't treat it as a second-class > > > citizen. > > > > > > Yeah, I don't want to treat folks on Windows as second class citizens by > > any means - but I wouldn't mi

[clang] [clang-tools-extra] Remove `StringLiteral` in favor of `StringRef` (PR #122366)

2025-01-13 Thread David Blaikie via cfe-commits
dwblaikie wrote: > Anyway, my main point was that there are people who care about performance on > Windows, so please don't treat it as a second-class citizen. Yeah, I don't want to treat folks on Windows as second class citizens by any means - but I wouldn't mind/hope we can treat folks who c

[clang] [clang-tools-extra] Remove `StringLiteral` in favor of `StringRef` (PR #122366)

2025-01-10 Thread David Blaikie via cfe-commits
dwblaikie wrote: > > Do we care about performance on MSVC ? > > Just responding to this point (although I accept it's moot in the current > context): yes we care about performance when building with MSVC - our > downstream toolchain is hosted on Windows. I'm sure we're not the only > Windows

[clang] Allow packing fields into tail padding of record fields (PR #122197)

2025-01-08 Thread David Blaikie via cfe-commits
@@ -803,10 +804,16 @@ bool ConstStructBuilder::Build(const InitListExpr *ILE, bool AllowOverwrite) { if (!AppendField(Field, Layout.getFieldOffset(FieldNo), EltInit, AllowOverwrite)) return false; - // After emitting a non-empty field

[clang] Allow packing fields into tail padding of record fields (PR #122197)

2025-01-08 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie commented: Thanks! Looks plausible to me, would be great to get @Michael137's take on it, since he's been looking at this sort of thing a fair bit lately. https://github.com/llvm/llvm-project/pull/122197 ___ cfe-commits ma

[clang] Allow packing fields into tail padding of record fields (PR #122197)

2025-01-08 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie edited https://github.com/llvm/llvm-project/pull/122197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Add "extend lifetime" flags and release note (PR #110000)

2025-01-08 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. https://github.com/llvm/llvm-project/pull/11 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC] Updating Debug Info generation for 'this' (PR #119445)

2024-12-17 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. https://github.com/llvm/llvm-project/pull/119445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [DebugInfo] Place local ODR-uniqued types in decl DISubprograms (PR #119001)

2024-12-13 Thread David Blaikie via cfe-commits
dwblaikie wrote: yeah, I worry this feels all a bit vague (that some things "suggest" other things, that root problems "seem to be" (& some fairly broad descriptions of what they seem to be) - not to nitpick your language, and I appreciate the clarity of the uncertainty, to be sure - but the u

[clang] [NFC] Updating Debug Info generation for 'this' (PR #119445)

2024-12-10 Thread David Blaikie via cfe-commits
dwblaikie wrote: any chance you could A/B test this on a bootstrap of clang, for instance? To help validate that this really is NFC/dead code? https://github.com/llvm/llvm-project/pull/119445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[clang] [llvm] [DebugInfo] Place local ODR-uniqued types in decl DISubprograms (PR #119001)

2024-12-10 Thread David Blaikie via cfe-commits
dwblaikie wrote: > > Maybe it's too big/complex a problem to try to think about, and incremental > > development should overrule here - but may be worth thinking about? > > Urgh. I'll admit that I hadn't thought that far ahead, I just wanted to get > the patch-series going again now that the r

[clang] [HLSL] Fix debug info generation for RWBuffer types (PR #119041)

2024-12-09 Thread David Blaikie via cfe-commits
@@ -2021,28 +2021,10 @@ llvm::DISubroutineType *CGDebugInfo::getOrCreateInstanceMethodType( // ThisPtr may be null if the member function has an explicit 'this' // parameter. if (!ThisPtr.isNull()) { -const CXXRecordDecl *RD = ThisPtr->getPointeeCXXRecordDecl(); -

[clang] [llvm] [DebugInfo] Place local ODR-uniqued types in decl DISubprograms (PR #119001)

2024-12-09 Thread David Blaikie via cfe-commits
dwblaikie wrote: No idea if this sufficiently overlaps with, or is orthogonal to the following, but: Currently we don't scope function-local types into the appropriate scope inside a function. It seems like associating types with their declaration may make this harder to address - how would w

[clang] Switch builtin strings to use string tables (PR #118734)

2024-12-05 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. This looks fine to me - though given the broad impact (to every target in clang) maybe @AaronBallman wants to weigh in. If you wanted to front-load the things like `getRecord(ID).Attributes` -> `getAttributesString(ID)` those things coul

[clang] [Clang] Enable -fextend-lifetimes at -Og (PR #118026)

2024-12-04 Thread David Blaikie via cfe-commits
@@ -1834,6 +1834,14 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, Opts.setInlining(CodeGenOptions::NormalInlining); } + // If we have specified -Og and have not explicitly set -fno-extend-lifetimes, + // then default to -fextend-li

[clang] [Clang] Add fake use emission to Clang with -fextend-lifetimes (PR #110102)

2024-12-04 Thread David Blaikie via cfe-commits
@@ -1353,6 +1353,19 @@ void CodeGenFunction::EmitLifetimeEnd(llvm::Value *Size, llvm::Value *Addr) { C->setDoesNotThrow(); } +void CodeGenFunction::EmitFakeUse(Address Addr) { + // We do not emit a fake use if we want to apply optnone to this function, + // even if we mig

[clang] [Clang] Add fake use emission to Clang with -fextend-lifetimes (PR #110102)

2024-12-03 Thread David Blaikie via cfe-commits
@@ -1353,6 +1353,19 @@ void CodeGenFunction::EmitLifetimeEnd(llvm::Value *Size, llvm::Value *Addr) { C->setDoesNotThrow(); } +void CodeGenFunction::EmitFakeUse(Address Addr) { + // We do not emit a fake use if we want to apply optnone to this function, + // even if we mig

[clang] [clang][codegen] Mention the invariant that LLVM demangler should be … (PR #117346)

2024-11-25 Thread David Blaikie via cfe-commits
dwblaikie wrote: (please don't merge PRs that haven't been approved, that's against LLVM practices/policies ( https://llvm.org/docs/CodeReview.html#code-review-workflow ), unless they weren't intended for pre-commit review in the first place (if they were only meant for presubmit checks)) htt

[clang] [clang-tools-extra] [flang] [lld] [lldb] [llvm] [NFC] Rename Option parsing related files from OptXYZ -> OptionXYZ (PR #110692)

2024-10-08 Thread David Blaikie via cfe-commits
dwblaikie wrote: Yeah, I think given how wide the change is and the speculative nature of the harm - probably best to abandon this for now. We can pick it up again if more folks speak up/more evidence is presented that it's problematic. https://github.com/llvm/llvm-project/pull/110692

[clang] [Clang] Add "extend lifetime" flags and release note (PR #110000)

2024-10-03 Thread David Blaikie via cfe-commits
@@ -211,6 +211,16 @@ New Compiler Flags only for thread-local variables, and none (which corresponds to the existing ``-fno-c++-static-destructors`` flag) skips all static destructors registration. +- The ``-fextend-lifetimes`` and ``-fextend-this-ptr`` flags have been ad

[clang] [Clang] Add "extend lifetime" flags and release note (PR #110000)

2024-10-03 Thread David Blaikie via cfe-commits
@@ -0,0 +1,12 @@ +// RUN: %clang_cc1 %s -emit-llvm -O2 -fextend-lifetimes -o - | FileCheck --check-prefixes=CHECK-ALL,CHECK-O2 %s +// RUN: %clang_cc1 %s -emit-llvm -O0 -fextend-lifetimes -o - | FileCheck --check-prefixes=CHECK-ALL,CHECK-O0 %s + +// Checks that we emit the functi

[clang] [Clang] Add "extend lifetime" flags and release note (PR #110000)

2024-10-03 Thread David Blaikie via cfe-commits
@@ -2217,6 +2217,11 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags, Opts.SanitizeTrap); + Opts.ExtendThisPtr = + Opts.OptimizationLevel > 0

[clang] [clang][DebugInfo] Revert to printing canonical typenames for template aliases (PR #110767)

2024-10-02 Thread David Blaikie via cfe-commits
dwblaikie wrote: Thanks @jimingham - yeah, that makes sense. My original argument for being OK with the de-canonicalization of typedefs was that they aren't load bearing anyway. But if lldb puts load on them, that assumption doesn't hold. Yeah, could check for angles or template parameter DIEs

[clang] [clang][DebugInfo] Revert to printing canonical typenames for template aliases (PR #110767)

2024-10-02 Thread David Blaikie via cfe-commits
dwblaikie wrote: Not quite sure how we get to caching and template specializations and pretty printers. If we're still producing the typedef-style DWARF for these alias template specializations - perhaps lldb could not cache pretty printers for typedefs? (I guess the pretty printers shouldn't

[clang] [DebugInfo] Correct the line attribution for IF branches (PR #108300)

2024-09-19 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. Fair enough https://github.com/llvm/llvm-project/pull/108300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [DebugInfo] Correct the line attribution for IF branches (PR #108300)

2024-09-12 Thread David Blaikie via cfe-commits
@@ -0,0 +1,45 @@ +// RUN: %clang_cc1 -debug-info-kind=limited -gno-column-info -triple=x86_64-pc-linux -emit-llvm %s -o - | FileCheck %s + +// The important thing is that the compare and the conditional branch have +// locs with the same scope (the lexical block for the 'if'). B

[clang] [DebugInfo] Correct the line attribution for IF branches (PR #108300)

2024-09-12 Thread David Blaikie via cfe-commits
@@ -0,0 +1,45 @@ +// RUN: %clang_cc1 -debug-info-kind=limited -gno-column-info -triple=x86_64-pc-linux -emit-llvm %s -o - | FileCheck %s + +// The important thing is that the compare and the conditional branch have +// locs with the same scope (the lexical block for the 'if'). B

[clang] [DebugInfo] Correct the line attribution for IF branches (PR #108300)

2024-09-12 Thread David Blaikie via cfe-commits
@@ -0,0 +1,45 @@ +// RUN: %clang_cc1 -debug-info-kind=limited -gno-column-info -triple=x86_64-pc-linux -emit-llvm %s -o - | FileCheck %s + +// The important thing is that the compare and the conditional branch have +// locs with the same scope (the lexical block for the 'if'). B

[clang] [clang][Sema] Fix diagnostic for function overloading in extern "C" (PR #106033)

2024-08-30 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. https://github.com/llvm/llvm-project/pull/106033 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Sema] Fix diagnostic for function overloading in extern "C" (PR #106033)

2024-08-30 Thread David Blaikie via cfe-commits
dwblaikie wrote: > This make sense to me, @dwblaikie wdyt? Seems OK to me https://github.com/llvm/llvm-project/pull/106033 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Assert non-null enum definition in CGDebugInfo::CreateTypeDefinition(const EnumType*) (PR #105556)

2024-08-26 Thread David Blaikie via cfe-commits
dwblaikie wrote: What static analysis tool flagged this? Any idea why this particular null dereference, when LLVM/Clang have loads of assumed-non-null pointers that are dereferenced in a great many places? https://github.com/llvm/llvm-project/pull/105556 ___

[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)

2024-08-20 Thread David Blaikie via cfe-commits
dwblaikie wrote: > Looking forward to trying this out! > > Should the new flag have some docs and maybe be mentioned in the release > notes, or do we think it's not ready for prime time yet for some reason? I'm /pretty/ neutral on that - it's got pretty clear behavior, etc (& in fact some cla

[clang] [lld] [llvm] [mlir] [NFC][IWYU] Update Support library with IWYU. (PR #102707)

2024-08-12 Thread David Blaikie via cfe-commits
dwblaikie wrote: > > The motivation is as usual IWYU and similar refactoring - to reduce build > > time and probablility of non-related source(s) recompile. > > I'm confused: as far as I know IWYU achieves the opposite of what you're > describing actually: it adds more includes than strictly n

[clang] [llvm] [Clang] Protect ObjCMethodList assignment operator against self-assignment (PR #97933)

2024-08-04 Thread David Blaikie via cfe-commits
dwblaikie wrote: Is there existing test coverage (like, if you add an assertion, instead of an if/return - do any tests hit the assertion)? or could you add some? https://github.com/llvm/llvm-project/pull/97933 ___ cfe-commits mailing list cfe-commits

[clang] [clang][CGDebugInfo] Don't generate an implicit 'this' parameter if one was specified explicitly (PR #100767)

2024-07-26 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/100767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 8dd5742 - Fix aarch64-ptrauth.c to avoid writing to cwd which might not be writeable

2024-07-25 Thread David Blaikie via cfe-commits
Author: David Blaikie Date: 2024-07-25T21:52:14Z New Revision: 8dd574236ccaa0a183278396cfec3068b832651a URL: https://github.com/llvm/llvm-project/commit/8dd574236ccaa0a183278396cfec3068b832651a DIFF: https://github.com/llvm/llvm-project/commit/8dd574236ccaa0a183278396cfec3068b832651a.diff LOG:

[clang] [Clang] Fix null pointer dereference in enum debug info generation (PR #97105)

2024-07-24 Thread David Blaikie via cfe-commits
@@ -98,3 +98,6 @@ enum E8 { A8 = -128, B8 = 127 } x8; // CHECK-NOT: DIFlagEnumClass // CHECK: !DIEnumerator(name: "A8", value: -128) +// Forward declaration of an enum class. +enum class Color : int; +// CHECK-NOT: !DICompositeType(tag: DW_TAG_enumeration_type, name: "Color" -

[clang] [llvm] Revert "[PS4/PS5][Driver][DWARF] Always emit .debug_aranges for SCE t… (PR #99711)

2024-07-22 Thread David Blaikie via cfe-commits
dwblaikie wrote: also, if you're sending a pull request just because it's convenient (via the web UI "revert" button) or to run presubmits but not for review, please include the skip-presubmit-approval label so it's clear something isn't getting committed without needed review https://github.

[clang] [llvm] Revert "Add source file name for template instantiations in -ftime-trace" (PR #99534)

2024-07-21 Thread David Blaikie via cfe-commits
dwblaikie wrote: (if you don't need a code review for a change, you can commit it directly without a pull request - if you're making a PR not for code review but for presubmit checks, please label to `skip-precommit-approval` to clarify that you aren't waiting for approval (and that the change

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

2024-07-16 Thread David Blaikie via cfe-commits
dwblaikie wrote: > > That's a pretty substantial policy change to propose, and this probably > > isn't the place to propose/discuss it. If that's your intent, probably best > > to take that up on discord. > > I am not proposing a policy change. I believe the current policy is aimed at > givin

[clang] [Clang] Fix null pointer dereference in enum debug info generation (PR #97105)

2024-07-16 Thread David Blaikie via cfe-commits
dwblaikie wrote: > Thanks @smanna12. I think this looks ok; returning null here does appear to > be consistent with other overloads of `CreateTypeDefinition` for `RecordType` > and `ObjCInterfaceType`. I agree with @Michael137 that it would be nice to > have an example that fails the added con

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

2024-07-14 Thread David Blaikie via cfe-commits
dwblaikie wrote: > Also, I think we presently have sufficient review bandwidth in clang proper, > even including the area of modules, so that any non-trivial change can be > reviewed, and I also propose that we abstain from trying to directly commit > those. That's a pretty substantial policy

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

2024-07-12 Thread David Blaikie via cfe-commits
dwblaikie wrote: > I think the appropriate tag for such commits would be NFCI, and should still > require PR and review. Clarifying a couple of things here 1) I tend to agree the patch might've been a bit subtle and maybe split up into smaller, independent parts (usual rules: [be an isolated c

[clang] [clang][CGRecordLayout] Remove dependency on isZeroSize (PR #96422)

2024-07-09 Thread David Blaikie via cfe-commits
@@ -185,6 +203,18 @@ CALL_AO(PackedMembers) // CHECK: call void @llvm.memcpy.p0.p0.i64({{.*}} align 1 {{.*}} align 1 {{.*}}i64 16, i1 {{.*}}) // CHECK: ret ptr +// WithEmptyField copy-assignment: +// CHECK-LABEL: define linkonce_odr nonnull align {{[0-9]+}} dereferenceable({

[clang] [Clang][Sema] Diagnose variable template explicit specializations with storage-class-specifiers (PR #93873)

2024-07-02 Thread David Blaikie via cfe-commits
dwblaikie wrote: > It looks like the presence of `static` on template variable specializations > makes difference in the namespace context: https://gcc.godbolt.org/z/WGsreqbz8 > > Specifically, the specializations not marked `static` result in an exported > variable. Thus, we have seemingly va

[clang] [Driver] Add -Wa, options --crel and --allow-experimental-crel (PR #97378)

2024-07-02 Thread David Blaikie via cfe-commits
@@ -0,0 +1,25 @@ +// RUN: not %clang -### -c --target=x86_64 -Wa,--crel %s 2>&1 | FileCheck %s --check-prefix=NOEXP + +// NOEXP: error: -Wa,--allow-experimental-crel must be specified to use -Wa,--crel. CREL is experimental and takes a non-standard section type code + +// RUN: %

[clang] [clang] Use std::make_unique (NFC) (PR #97176)

2024-06-29 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. https://github.com/llvm/llvm-project/pull/97176 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Use std::make_unique (NFC) (PR #97176)

2024-06-29 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie edited https://github.com/llvm/llvm-project/pull/97176 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Clang: Add warning flag for storage class specifiers on explicit specializations (PR #96699)

2024-06-27 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie closed https://github.com/llvm/llvm-project/pull/96699 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Clang: Add warning flag for storage class specifiers on explicit specializations (PR #96699)

2024-06-27 Thread David Blaikie via cfe-commits
dwblaikie wrote: > Minor nit: Probably don't need to explicitly define a diagnostic group, since > it's only ever used for this diagnostic. Not sure what the conventions are - but I rather like the normalization of having the explicit group, better chance of finding the group/reusing it maybe?

[clang] [Clang] Static and explicit object member functions with the same parameter-type-lists (PR #93430)

2024-06-26 Thread David Blaikie via cfe-commits
dwblaikie wrote: @cor3ntin ping on this? https://github.com/llvm/llvm-project/pull/93430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)

2024-06-26 Thread David Blaikie via cfe-commits
dwblaikie wrote: Oh, this code was touched really recently in 66df6141659375e738d9b9b74bf79b2317576042 (@augusto2112 ) (see notes in previous comments about how this code should be generalized) https://github.com/llvm/llvm-project/pull/93809 ___ cfe-

[clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)

2024-06-26 Thread David Blaikie via cfe-commits
dwblaikie wrote: Here's the smallest patch that would put explicit alignment on any packed structure: ``` diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index a072475ba770..bbb13ddd593b 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CG

[clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)

2024-06-26 Thread David Blaikie via cfe-commits
dwblaikie wrote: The other effects of `packed` are encoded (the changes to the member offsets) but that's not the only effect, and we shouldn't use that effect (which isn't guaranteed to be observable - as in the case of "packed struct with a single member/that just happens to not have padding

[clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)

2024-06-26 Thread David Blaikie via cfe-commits
dwblaikie wrote: oh, slight misquote above - `aligned(1)` is a no-op, as that attribute can't reduce the alignment... But I think my argument still stands, roughly as - if increases in alignment are explicitly specified, decreases in alignment should be too. https://github.com/llvm/llvm-proj

[clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)

2024-06-26 Thread David Blaikie via cfe-commits
dwblaikie wrote: Ah, I think this right solution to /this/ case is that the compiler should be emitting the alignment for the structure? Like there's no way for the debugger to know that this structure is underaligned at the moment: ``` struct __attribute__((packed)) t1 { int i; }; ``` Which

[clang] Clang: Add warning flag for storage class specifiers on explicit specializations (PR #96699)

2024-06-25 Thread David Blaikie via cfe-commits
dwblaikie wrote: > Should we add a test for this flag? Something around existing tests in > clang/test/CXX/temp/temp.spec/temp.expl.spec/p2-20.cpp and > clang/test/CXX/dcl.dcl/dcl.spec/dcl.stc/p1.cpp. Oh, right, I did have a new test I meant to add, but that's a better place for it - so I've

[clang] Clang: Add warning flag for storage class specifiers on explicit specializations (PR #96699)

2024-06-25 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie updated https://github.com/llvm/llvm-project/pull/96699 >From a539afc7b81502ffcab7028bfe8266b8e32951d9 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Tue, 25 Jun 2024 21:02:50 + Subject: [PATCH 1/2] Clang: Add warning flag for storage class specifiers on ex

[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-25 Thread David Blaikie via cfe-commits
@@ -86,6 +86,8 @@ DYNAMIC_TAG(RELRSZ, 35) // Size of Relr relocation table. DYNAMIC_TAG(RELR, 36)// Address of relocation table (Relr entries). DYNAMIC_TAG(RELRENT, 37) // Size of a Relr relocation entry. +DYNAMIC_TAG(CREL, 38) // CREL relocation table + --

[clang] [Clang][Sema] Diagnose variable template explicit specializations with storage-class-specifiers (PR #93873)

2024-06-25 Thread David Blaikie via cfe-commits
dwblaikie wrote: Sent a patch to add a warning flag for the warning this patch uses: https://github.com/llvm/llvm-project/pull/96699 With that, we could disable the warning during the compiler migration, decoupling compiler migration from code cleanup. https://github.com/llvm/llvm-project/pul

[clang] Clang: Add warning flag for storage class specifiers on explicit specializations (PR #96699)

2024-06-25 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie created https://github.com/llvm/llvm-project/pull/96699 With the recent fix for this situation in class members (#93873) (for which the fixed code is invalid prior to this patch - making migrating code difficult as it must be in lock-step with the compiler migration,

[clang] [Clang] Static and explicit object member functions with the same parameter-type-lists (PR #93430)

2024-06-24 Thread David Blaikie via cfe-commits
dwblaikie wrote: Hit an msan use-of-uninitialized on an internal use case (a tool that uses Clang via API). the test was compiling this source code: ``` #include namespace proto2 { struct Message {}; } // namespace proto2 struct FakeProto : proto2::Message { const std::string& getter() cons

[clang] Add support for builtin_verbose_trap (PR #79230)

2024-06-24 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. https://github.com/llvm/llvm-project/pull/79230 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

2024-06-24 Thread David Blaikie via cfe-commits
@@ -28,7 +29,7 @@ namespace llvm { } // Prefix of the name of the artificial inline frame. -#define CLANG_TRAP_PREFIX "__clang_trap_msg" +inline constexpr llvm::StringRef CLANG_TRAP_PREFIX = "__clang_trap_msg"; dwblaikie wrote: The name should be changed to u

[clang] Add support for builtin_verbose_trap (PR #79230)

2024-06-24 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie edited https://github.com/llvm/llvm-project/pull/79230 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Driver] Expose `-fno-eliminate-unused-debug-types` to clang-cl (PR #95259)

2024-06-24 Thread David Blaikie via cfe-commits
dwblaikie wrote: > > I will say, `-fno-eliminate-unused-debug-types` is a really heavy hammer > > that makes debug info much larger - and my understanding was that games > > tended to have trouble with large debug builds, so I'd be surprised if this > > was a great path forward. > > Absolutel

[clang] Add support for builtin_verbose_trap (PR #79230)

2024-06-22 Thread David Blaikie via cfe-commits
https://github.com/dwblaikie approved this pull request. Looks OK - one minor nit, I'd probably avoid defining CLANG_TRAP_PREFIX as a macro, but instead as an inline or some other form of constant https://github.com/llvm/llvm-project/pull/79230 ___ cf

[clang] Add support for builtin_verbose_trap (PR #79230)

2024-06-22 Thread David Blaikie via cfe-commits
@@ -27,6 +27,9 @@ namespace llvm { } } +// Prefix of the name of the artificial inline frame. +#define CLANG_TRAP_PREFIX "__clang_trap_msg" dwblaikie wrote: inline StringRef? usual reasons to avoid macros, etc https://github.com/llvm/llvm-project/pull/7923

[clang] [Clang][Driver] Expose `-fno-eliminate-unused-debug-types` to clang-cl (PR #95259)

2024-06-22 Thread David Blaikie via cfe-commits
dwblaikie wrote: I will say, `-fno-eliminate-unused-debug-types` is a really heavy hammer that makes debug info much larger - and my understanding was that games tended to have trouble with large debug builds, so I'd be surprised if this was a great path forward. Do you have a small example o

[clang] [Clang][Driver] Expose `-fno-eliminate-unused-debug-types` to clang-cl (PR #95259)

2024-06-22 Thread David Blaikie via cfe-commits
dwblaikie wrote: Yes, the initializers do have to be lazyily evaluated to be a conforming C++ compiler. eg: this code must compile without error so far as I understand: ``` template struct t1 { static constexpr int x = 3 / Value; }; t1<0> v1; ``` Though it seems MSVC doesn't implement this co

[clang] Add support for builtin_verbose_trap (PR #79230)

2024-06-20 Thread David Blaikie via cfe-commits
dwblaikie wrote: I think my last comment/question is still open? How/why did the symbol name end up dropping any llvm/clang component to avoid collisions with other names? https://github.com/llvm/llvm-project/pull/79230 ___ cfe-commits mailing list cf

  1   2   3   4   5   6   7   8   9   10   >