[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

dblaikie added a comment. In D137058#4030331 , @ChuanqiXu wrote: > @dblaikie would you like to take a look at this? Yep, this is still on my radar - sorry for the delays. I'll get to it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137058/new/

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

dblaikie added a comment. Ah, so the intent is that this causes these indirect args to be handled the same way as other arguments at -O0 - placed in an alloca, eg: struct t1 { t1(const t1&); }; void f1(int, int); void f2(t1 v, int x) { f1(3, 4); } 0x0033: DW_TAG_form

[PATCH] D139986: [clang][TypePrinter] Teach isSubstitutedDefaultArgument about integral types

dblaikie added a comment. In D139986#4040208 , @erichkeane wrote: > In D139986#4040185 , @ldionne wrote: > >> @dblaikie >> >> We added the libc++ tests to the Clang pre-commit CI after discussing with >> @erichk

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

dblaikie added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5733-5735 + if (!AtTopLevel && isa(JA) && + JA.getType() == types::TY_ModuleFile && + C.getArgs().hasArg(options::OPT_fmodule_output) && These conditions might be shared with the p

[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

dblaikie added a comment. This is a bit out of left field, but these comments reminded me of something - a long time ago I was working on fixing some Clang layering to potentially use explicit modules more in our internal build (& then hopefully in the upstream/external build) and one major hol

[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

dblaikie added a subscriber: rsmith. dblaikie added a comment. I realize I got this jumbled up and the thread about "why do we need to name things" is meant to be over in D137059 (sorry @ben.boeckel :/ I know this is all confusing to follow at the best of time

[PATCH] D137059: [Driver] [Modules] Introduce -fsave-std-c++-module-file= to specify the path of the module file (2/2)

dblaikie added a subscriber: rsmith. dblaikie added a comment. In D137059#3899339 , @ben.boeckel wrote: > There is another motivating factor for 1-phase: the build graph is far > simpler. With 2-phase, CMake will have to write out rules to perform: > >

[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

dblaikie added a comment. > What was the objection to "-fc++-module-filename[=]" ? I guess it reads a bit awkwardly when you aren't providing the filename/want the default filename? > GCC has "-fmodule-only" Hmm, I don't mind that too much (& as you say, '-fobject-only' - though that flag is

[PATCH] D136554: Implement CWG2631

dblaikie added subscribers: rsmith, dblaikie. dblaikie added a comment. fwiw, @rsmith came up with a crasher reproducer from this patch here: template struct F { template F(const U&) {} }; struct A { static constexpr auto x = [] {}; F f = x; }; void f(A a = A())

[PATCH] D136554: Implement CWG2631

dblaikie added a comment. Looks like it's distinct from the other reported crash, this one crashes with: clang-16: /usr/local/google/home/blaikie/dev/llvm/src/clang/lib/CodeGen/CGExpr.cpp:2811: clang::CodeGen::LValue clang::CodeGen::CodeGenFunction::EmitDeclRefLValue(const clang::DeclRefExpr

[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

dblaikie added a subscriber: aaron.ballman. dblaikie added a comment. @aaron.ballman does this seem OK? (this was based on my suggestion in the related review linked in the description) It probably needs tests in clang too - not sure if there's an opportunity to use a unit test to simplify acce

[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

dblaikie added a comment. In D137583#3917706 , @aaron.ballman wrote: >> ...we expect template params to be fully qualified when comparing them for >> simple template names > > So lldb is not inspecting the AST, they're doing reflection (of a sort) on >

[PATCH] D137609: [C++20] [Modules] Remove unmaintained header modules

dblaikie added a comment. > I'm not aware of anyone using this mode, but please wait for responses from > Google and Meta people to verify that. Best understanding on the Google side is we aren't using this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137609/new/ https://reviews.llv

[PATCH] D137885: [modules] Support zstd in .pcm file

dblaikie added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:1456 + const llvm::compression::Format F = + Blob.size() >= 2 && memcmp(Blob.data(), "\x1f\x8b", 2) == 0 + ? llvm::compression::Format::Zlib Worth a comme

[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

dblaikie added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5541-5542 + // + // FIXME: Do we need to handle the case that the calculated output is + // conflicting with the specified output file or the input file? + if (!AtTopLevel && isa(JA) && D

[PATCH] D135488: [codegen] Display stack layouts in console

dblaikie added a comment. Any chance we could squirrel the info away (I assume there's a reason we can't compute the info where the warn-stack-size LLVM feature is implemented in PrologEpilogInserter.cpp) somewhere, and emit it as part of the frame-larger-than/warn-stack-size diagnostic? (also

[PATCH] D135488: [codegen] Display stack layouts in console

dblaikie added a comment. > as a more general diagnostic output, like the other printing passes. As an aside: I don't think any "printing pass" is designed to be used beyond LLVM compiler engineers - they're implementation details of the compiler, even/much moreso than the optimization remarks

[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

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

[PATCH] D135488: [codegen] Display stack layouts in console

dblaikie added a comment. In D135488#3928963 , @paulkirth wrote: > In D135488#3928831 , @dblaikie > wrote: > >> Any chance we could squirrel the info away (I assume there's a reason we >> can't compute the info

[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Couple of minor fixes for the test, but otherwise seems fine. Comment at: clang/test/Driver/save-std-c++-module-file.cpp:1-4 +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN:

[PATCH] D135488: [codegen] Display stack layouts in console

dblaikie added a comment. > This pass prints a TON of (helpful) information...we have a lot of > -Wframe-larger-than= instances triggered in our codebase...I think having > this on by default would blow our logs significantly. That's why it might be > nice to have a Note suggest a flag (default

[PATCH] D137059: [Driver] [Modules] Introduce -fsave-std-c++-module-file= to specify the path of the module file (2/2)

dblaikie added a comment. @ben.boeckel >> Plus the other compilers offer controls over it; why does Clang have to be >> different? > > Which compilers/flags are you referring to? Arguments from compatibility with > GCC are relatively easy to make (though I still have more hesitance for these >

[PATCH] D138247: PR58819: Correct linkage and mangling of lambdas in inline static member initializers

dblaikie created this revision. dblaikie added reviewers: clang-vendors, rsmith, probinson, rjmccall. Herald added a project: All. dblaikie requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. https://llvm.org/pr58819 - clang is giving an extern

[PATCH] D141826: [WIP][clang][TemplateBase] Add IsDefaulted bit to TemplateArgument

dblaikie added a comment. In D141826#4072609 , @Michael137 wrote: > In D141826#4059088 , @erichkeane > wrote: > >> In D141826#4059073 , @Michael137 >> wrote: >> >>> In

[PATCH] D140250: Define NULL in its own header

dblaikie added inline comments. Comment at: llvm/test/DebugInfo/Generic/assignment-tracking/sroa/unspecified-var-size.ll:37 !7 = !DIFile(filename: "clang/12.0.0/include/__stddef_max_align_t.h", directory: "/") -!8 = !DICompositeType(tag: DW_TAG_structure_type, file: !7, line:

[PATCH] D140250: Define NULL in its own header

dblaikie added inline comments. Comment at: llvm/test/DebugInfo/Generic/assignment-tracking/sroa/unspecified-var-size.ll:37 !7 = !DIFile(filename: "clang/12.0.0/include/__stddef_max_align_t.h", directory: "/") -!8 = !DICompositeType(tag: DW_TAG_structure_type, file: !7, line:

[PATCH] D142675: [clang][CGDebugInfo] emit DW_LANG_C_plus_plus_{20|17} DW_LANG_C17

dblaikie added a comment. This was discussed in D138597 & we decided not to implement these due to the risk of breaking existing v5 clients that might not know about the post-v5-release codes. Implementing the v6 codes as an extension might be an option, or em

[PATCH] D142861: [Clang] avoid relying on StringMap iteration order when roundtripping -analyzer-config

dblaikie added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:881 + // Sort options by key to avoid relying on StringMap iteration order. + SmallVector, 4> SortedConfigOpts; for (const auto &C : Opts.Config) { jansvoboda11 wrote: > ja

[PATCH] D142861: [Clang] avoid relying on StringMap iteration order when roundtripping -analyzer-config

dblaikie added a comment. (heh, don't mind my feedback being duplicate - didn't refresh before submitting - glad other folks got to it before me! :) ) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142861/new/ https://reviews.llvm.org/D142861

[PATCH] D142800: [Clang][Diagnostic] Add `-Wcomparison-op-parentheses` to warn on chained comparisons

dblaikie added a comment. > I'd be interested to see the fixit-hints for the first bit, also to see how > others feel about it here. My 2c is that `-Wparentheses` is already a very stylistic warning (even correct code, without knowing about this esoteric/specific suppression style of adding pa

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

dblaikie added a comment. In D141625#4094742 , @steven_wu wrote: > In D141625#4067225 , @dblaikie > wrote: > >> In D141625#4066961 , @steven_wu >> wrote: >> >>> No, reve

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

dblaikie added a comment. In D141625#4094942 , @steven_wu wrote: > In D141625#4094837 , @dblaikie > wrote: > >> In D141625#4094742 , @steven_wu >> wrote: >> >>> In D1416

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

dblaikie added a comment. In D124351#4097110 , @cor3ntin wrote: > In D124351#4093840 , @nikic wrote: > >> FYI this causes a minor compile-time regression (around 0.35% on 7zip at >> `O0`): >> http://llvm-compile

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

dblaikie added a comment. In D141625#4095197 , @steven_wu wrote: >> Sorry, I'm still not really following - OK, sounds like you're saying this >> test does fail at HEAD/without this patch in reverse iteration mode, and is >> a bit larger than would be m

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

dblaikie added a comment. In D141451#4095303 , @nickdesaulniers wrote: > In D141451#4064298 , @dblaikie > wrote: > >> Right - I was thinking more, as above, about directly using the existing >> metadata generat

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

dblaikie added a comment. In D141625#4100660 , @akyrtzi wrote: >> would be great to test it more in a semantic way if possible > > Keep in mind that the specific order of the decls doesn't matter for the > purposes of this test, what matters is that the

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

dblaikie added a comment. In D141625#4100762 , @steven_wu wrote: > I don't think it is feasible with current tool to write a test that check > semantically the order of decls in clang module. (Let me know if that was > wrong). The current test already u

[PATCH] D141450: [Clang][cc1] Add -fno-modules-local-submodule-visibility to override the default

dblaikie added a comment. > This is a problem because some existing ObjectiveC code is not compatible > with LSV Hmm, how is that true? Does this code only build with Clang Header Modules enabled, and can't build without that? (if it could build without it, I don't know why it'd need LSV, if I

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

dblaikie added a comment. In D141381#4043985 , @fdeazeve wrote: > In D141381#4040639 , @probinson > wrote: > >> To get data about the code size impact, people typically build some large >> codebase with/without

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

dblaikie added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5736 + C.getArgs().hasArg(options::OPT_fmodule_output) && + C.getArgs().hasArg(options::OPT_o)) { +SmallString<128> OutputPath; ChuanqiXu wrote: > dblaikie wrote: > > Is there s

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

dblaikie added a comment. hmm, did we/do we ever motivate backend diagnostics using the debug info locations? I thought we did in some cases/ways - maybe when the frontend isn't available (eg: when compiling from LLVM IR). It'd be nice not to invent a new way of tracking inlining separate from

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

dblaikie added a comment. In D140860#4044937 , @aaron.ballman wrote: > In D140860#4031872 , @dblaikie > wrote: > >> The risk now is that this might significantly regress/add new findings for >> this warning tha

[PATCH] D139774: [libclang] Add API to set temporary directory location

dblaikie added a comment. In D139774#4044258 , @aaron.ballman wrote: > In D139774#4043886 , @vedgy wrote: > >> In D139774#4041308 , >> @aaron.ballman wrote: >> >>> Is yo

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

dblaikie added a comment. In D141381#4045215 , @rjmccall wrote: > That's about what I would expect. One or two extra instructions per argument > that are trivially removed in debug builds. Very small overall impact > because there just aren't very man

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

dblaikie added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5736 + C.getArgs().hasArg(options::OPT_fmodule_output) && + C.getArgs().hasArg(options::OPT_o)) { +SmallString<128> OutputPath; tahonermann wrote: > dblaikie wrote: > > ChuanqiX

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

dblaikie added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5736 + C.getArgs().hasArg(options::OPT_fmodule_output) && + C.getArgs().hasArg(options::OPT_o)) { +SmallString<128> OutputPath; ChuanqiXu wrote: > dblaikie wrote: > > tahonerman

[PATCH] D139774: [libclang] Add API to set temporary directory location

dblaikie added a comment. In D139774#4046308 , @vedgy wrote: > In D139774#4045361 , @dblaikie > wrote: > >> 1. Should clang be doing something better with these temp files anyway, no >> matter the directory they

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

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

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. In D140860#4047632 , @hazohelet wrote: > In D140860#4047534 , @aaron.ballman > wrote: > >> In D140860#40

[PATCH] D141450: [Clang][cc1] Add -fno-modules-local-submodule-visibility to override the default

dblaikie added a comment. In D141450#4049026 , @Bigcheese wrote: > In D141450#4044680 , @dblaikie > wrote: > >>> This is a problem because some existing ObjectiveC code is not compatible >>> with LSV >> >> Hmm,

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. I really don't think this is the right thing to do - the Split DWARF code, for instance, has support for GPU bundling that's missing in the module file naming code, which seems likely to b

[PATCH] D140423: [WIP][clang] Add PrintingPolicy callback for identifying default template arguments

dblaikie added a comment. In D140423#4051262 , @aaron.ballman wrote: >> Add something like a bool IsDefaulted somewhere in Clang, e.g., in >> TemplateArgument and consult it from the TypePrinter. This would be much >> simpler but requires adding a fiel

[PATCH] D141450: [Clang][cc1] Add -fno-modules-local-submodule-visibility to override the default

dblaikie added a comment. In D141450#4052210 , @jansvoboda11 wrote: > In D141450#4049049 , @dblaikie > wrote: > >> Perhaps I've got things confused, but my understanding of LSV was that it >> prevented other he

[PATCH] D140423: [WIP][clang] Add PrintingPolicy callback for identifying default template arguments

dblaikie added a comment. In D140423#4052540 , @erichkeane wrote: > A Class template instantiation SHOULD have its link back to the class > template, and should be able to calculate whether the template argument is > defaulted, right? For a normal/cla

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

dblaikie added a comment. Yeah, might be nice to document where the instability comes from - if it comes from a DenseMap or similar, then a test that fails either in forward or reverse iteration mode would be nice to have. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D141310: [clang] add -Wcompare-function-pointers

dblaikie added a comment. @adriandole do you plan to deploy this in a codebase? Have you tried it on a codebase already? I'd worry this would just be too noisy, and there's probably enough benign pointer comparisons that'll never hit the ICF false-equality situation (eg: putting some callbacks

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

dblaikie added a comment. In D137058#4055275 , @ChuanqiXu wrote: > In D137058#4050188 , @dblaikie > wrote: > >> I really don't think this is the right thing to do - the Split DWARF code, >> for instance, has sup

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

dblaikie added a comment. In D141625#4053067 , @steven_wu wrote: > @akyrtzi has the good idea. It is really hard to control `Decl*` to get values > to get an unstable iteration order from the small tests, going beyond 32 decls > to get out of SmallPtrSet'

[PATCH] D141826: [WIP][clang][TemplateBase] Add IsDefaulted bit to TemplateArgument

dblaikie added a comment. In D141826#4059088 , @erichkeane wrote: > In D141826#4059073 , @Michael137 > wrote: > >> In D141826#4058866 , @erichkeane >> wrote: >> >>> Thi

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

dblaikie added a comment. In D141381#4056661 , @fdeazeve wrote: > In hindsight, this should have been obvious. > While SROA will not touch this: > > define @foo(ptr %arg) { > call void @llvm.dbg.declare(%arg, [...], metadata !DIExpression()) > > It

[PATCH] D141310: [clang] add -Wcompare-function-pointers

dblaikie added a comment. In D141310#4060069 , @aaron.ballman wrote: > In D141310#4054351 , @dblaikie > wrote: > >> @adriandole do you plan to deploy this in a codebase? Have you tried it on a >> codebase alrea

[PATCH] D141310: [clang] add -Wcompare-function-pointers

dblaikie added a comment. But I don't mean to suggest I should be a decider/veto here - it's cheap to maintain/no big deal, so maybe that's fine - but for myself, having at least some large scale customer with existing experience testing the warning and a strong commitment/motivation to keep us

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

dblaikie added a subscriber: colinl. dblaikie added a comment. In D141625#4059486 , @steven_wu wrote: > @dblaikie Do we have any bots running reverse iteration? Hmm, not that I can see/find at a quick glance, which is unfortunate. @mgrang Are you still

[PATCH] D141310: [clang] add -Wcompare-function-pointers

dblaikie added a comment. In D141310#4062776 , @adriandole wrote: > @dblaikie, we would use this warning in Chrome OS. Ah, good to know! > We use `icf=all` and have encountered bugs caused by function pointer > comparisons. & the savings are worth it

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

dblaikie added a comment. In D141451#4063151 , @nickdesaulniers wrote: > In D141451#4045658 , @efriedma > wrote: > >> clang has a "LocTrackingOnly" setting for debug info, which emits DILocation >> info into th

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

dblaikie added a comment. In D141451#4063504 , @nickdesaulniers wrote: > In D141451#4063335 , @dblaikie > wrote: > >> In D141451#4063151 , >> @nickdesaulniers wrote: >>

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

dblaikie added subscribers: JDevlieghere, aprantl. dblaikie added a comment. In D141451#4063582 , @nickdesaulniers wrote: > In D141451#4063519 , @dblaikie > wrote: > >> In D141451#4063504

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

dblaikie added a comment. In D141625#4066362 , @steven_wu wrote: > @dblaikie Do you have any objection for committing the patch as it is? I > don't think reverse iteration test is a proper way to test for this specific > bug. I think reverse iteration

[PATCH] D141310: [clang] add -Wcompare-function-pointers

dblaikie added a comment. >>> It's not that noisy compiling clang (eight hits). >> >> Good to know - I'm surprised it's that low. >> >> Is there some idiom we can use/document/recommend for people to use when the >> warning is a false positive? (when the user is confident the functions won't >>

[PATCH] D139774: [libclang] Add API to set temporary directory location

dblaikie added a comment. In D139774#4065753 , @aaron.ballman wrote: > In D139774#4063131 , @vedgy wrote: > >> After a discussion under the corresponding KDevelop merge request, I can see >> 4-6 alternative ways

[PATCH] D139774: [libclang] Add API to set temporary directory location

dblaikie added a comment. In D139774#4066574 , @aaron.ballman wrote: > In D139774#4066519 , @dblaikie > wrote: > >> I've mixed feelings about this, but yeah, I guess the issues with global >> state are pretty u

[PATCH] D139774: [libclang] Add API to set temporary directory location

dblaikie added a comment. Don't let me hold this up - I think it all feels a bit too ad-hoc for my own preferences (feels like there should be fairly general solutions to this - rather than playing whack-a-mole on only the biggest temporary files - both in terms of the options KDevelop develope

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

dblaikie added a comment. In D141625#4066681 , @steven_wu wrote: > In D141625#4066466 , @dblaikie > wrote: > >> In D141625#4066362 , @steven_wu >> wrote: >> >>> @dblaiki

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

dblaikie added a comment. In D141625#4066961 , @steven_wu wrote: > No, reverse iteration will not break diff test for a small number of decls. > Everything will be in reverse order so it is the same. Hmm, I'm not sure I'm following why that is - could y

[PATCH] D139774: [libclang] Add API to set temporary directory location

dblaikie added a comment. In D139774#4069014 , @aaron.ballman wrote: > In D139774#4066920 , @dblaikie > wrote: > >> Don't let me hold this up - I think it all feels a bit too ad-hoc for my own >> preferences (f

[PATCH] D142268: [clang][DebugInfo] Don't canonicalize names in template argument list for alias templates

dblaikie added a comment. So @Michael137 and I talked about this offline, and a few extra details: - Generally it's important that types have identical names. Templates try to do this, but get it wrong in a bunch of ways (& certainly between GCC and Clang we get it different in a bunch of ways

[PATCH] D142268: [clang][DebugInfo] Don't canonicalize names in template argument list for alias templates

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

[PATCH] D142358: Opting out of Clang 15 ABI Changes for AIX and z/OS

dblaikie added inline comments. Comment at: clang/docs/ReleaseNotes.rst:822 classified such types as non-POD (for the purposes of Itanium ABI). Clang now - matches the gcc behavior (except on Darwin and PS4). You can switch back to + matches the gcc behavior (except on Darwi

[PATCH] D142358: Opting out of Clang 15 ABI Changes for AIX and z/OS

dblaikie added inline comments. Comment at: clang/test/SemaCXX/class-layout.cpp:18 // expected-no-diagnostics +#if !defined(__MVS__) && !defined(_AIX) What's the reason this part is #ifdef'd out? Does it contain code that's unsupported on these platforms? C

[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] D111521: [DebugInfo] Mark OpenMP generated functions as artificial

dblaikie added a comment. In D111521#3584439 , @alok wrote: > Re-based and updated to include one negative testcase. > I could not find a test for VarDecl with DynamicInitKind::NoStub. There are > constructors for other DynamicInitKind but not for NoStub

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

dblaikie added a comment. In D123319#3546535 , @dblaikie wrote: > In D123319#3532811 , @dblaikie > wrote: > >> Ping > > @aprantl thoughts on this? Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[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] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

dblaikie added a comment. In D123319#3593644 , @aprantl wrote: > In D123319#3517966 , @dblaikie > wrote: > >> In D123319#3506745 , @shafik wrote: >> >>> >> >> What does

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

dblaikie added subscribers: cmtice, dblaikie. dblaikie added a comment. Herald added a subscriber: steakhal. Looks like this got committed in 886715af962de2c92fac4bd37104450345711e4a though it was missing the `Differential Rev

[PATCH] D126334: Move GCC-compatible pod-packing change to v15/old behavior available at v14 and below

dblaikie created this revision. dblaikie added a reviewer: tstellar. Herald added a project: All. dblaikie requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Since this didn't make it into the v14 release - anyone requesting the v14 ABI should

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

dblaikie added inline comments. Comment at: clang/docs/ReleaseNotes.rst:239-243 +- GCC doesn't pack non-POD members in packed structs unless the packed + attribute is also specified on the member. Clang historically did perform + such packing. Clang now matches the gcc behavior

[PATCH] D126334: Move GCC-compatible pod-packing change to v15/old behavior available at v14 and below

This revision was automatically updated to reflect the committed changes. Closed by commit rGe59f648d698e: Move GCC-compatible pod-packing change to v15/old behavior available at v14 and… (authored by dblaikie). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

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

dblaikie added a comment. @rsmith I had a few responses to your last round of review here - could you have a look through them/see if this is the right way to go, or if more refactoring would be suitable? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

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

dblaikie updated this revision to Diff 431869. dblaikie added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118511/new/ https://reviews.llvm.org/D118511 Files: clang/include/clang/Basic/DiagnosticASTKinds.td clang/include/cla

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

dblaikie updated this revision to Diff 431871. dblaikie added a comment. Herald added a subscriber: MaskRay. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 Files: clang/include/clang/Basic/LangO

[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] D102122: Support warn_unused_result on typedefs

dblaikie added a comment. Herald added a project: All. In D102122#3344317 , @dblaikie wrote: > 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

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

dblaikie added inline comments. Comment at: clang/docs/ReleaseNotes.rst:239-243 +- GCC doesn't pack non-POD members in packed structs unless the packed + attribute is also specified on the member. Clang historically did perform + such packing. Clang now matches the gcc behavior

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

dblaikie added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + probinson wrote: > Does this mean `-fclang-abi-compat` will override the PS4/Darwin special > case? I think we don't want to do that

[PATCH] D102122: Support warn_unused_result on typedefs

dblaikie updated this revision to Diff 432091. dblaikie added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102122/new/ https://reviews.llvm.org/D102122 Files: clang/include/clang/Basic/Attr.td clang/lib/AST/Expr.cpp clang/

[PATCH] D102122: Support warn_unused_result on typedefs

dblaikie updated this revision to Diff 432131. dblaikie added a comment. Separate out clang:warn_unused_result so typedef support can be added to only that spelling. This has one lingering issue (because it still currently instantiates both the clang:warn_unused_result as the same *Attr class a

[PATCH] D102122: Support warn_unused_result on typedefs

dblaikie added a comment. Sorry, with all the layers in the previous messages I'm not sure what the summary is. Sounds like the summary is "this is OK to continue work in the direction of supporting [[clang::warn_unused_result]] (only that spelling) for typedefs in C and C++ (for compatibility

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

dblaikie added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + probinson wrote: > dblaikie wrote: > > probinson wrote: > > > Does this mean `-fclang-abi-compat` will override the PS4/Darwin special

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

dblaikie added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + probinson wrote: > dblaikie wrote: > > probinson wrote: > > > dblaikie wrote: > > > > probinson wrote: > > > > > Does this mean `-fcla

<    6   7   8   9   10   11   12   13   14   15   >