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

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rC359235: Skip type units/type uniquing when we know we're only emitting the type once… (authored by dblaikie, committed by

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

dblaikie added a comment. Seems like the right thing would be for the DWARF code that wants a rendered type name to pass its own printing policy, rather than changing some relatively global one. (though also I have my doubts about the whole approach - macro expansion can change the line number

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

dblaikie added a comment. In D38061#1484568 , @wenlei wrote: > In D38061#1484486 , @dblaikie wrote: > > > Seems like the right thing would be for the DWARF code that wants a > > rendered type name to pass its own p

[PATCH] D61357: SemaOverload: Complete candidates before emitting the error, to ensure diagnostics emitted (or suppressed) during completion don't interfere with the overload notes

dblaikie created this revision. dblaikie added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. Because diagnostics and their notes are not connected at the API level, if the error message for an overload is emitted, then the overload candidates are comple

[PATCH] D61357: SemaOverload: Complete candidates before emitting the error, to ensure diagnostics emitted (or suppressed) during completion don't interfere with the overload notes

dblaikie added a comment. Oh, @rsmith - if there's any better/different testing (if you can figure out how to reduce the test case down further, now that we know the cause - or if you'd like testing for other codepaths I've touched in this patch) I'm all ears. (also naming/API wrangling - my ch

[PATCH] D61408: Use primary template parameter names for variable template debug info

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Seems totally reasonable to me & it's what we do for other templates (function and class), it seems. Might want to check alias templates too? Repository: rG LLVM Github Monorepo CHANG

[PATCH] D61444: Do not warn on switches over enums that do not use [[maybe_unused]] enumerators

dblaikie created this revision. dblaikie added a reviewer: aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. PR36231, [dcl.attr.unused]p3 Repository: rC Clang https://reviews.llvm.org/D61444 Files: lib/Sema/SemaStmt.cpp test/CXX/dcl.dcl/dcl.attr/dcl.at

[PATCH] D61408: Use primary template parameter names for variable template debug info

dblaikie added a comment. In D61408#1487030 , @dblaikie wrote: > Seems totally reasonable to me & it's what we do for other templates > (function and class), it seems. > > Might want to check alias templates too? Ah, nevermind - alias templates can't be

[PATCH] D61444: Do not warn on switches over enums that do not use [[maybe_unused]] enumerators

This revision was automatically updated to reflect the committed changes. Closed by commit rC359800: Do not warn on switches over enums that do not use [[maybe_unused]] enumerators (authored by dblaikie, committed by ). Changed prior to commit: https://reviews.llvm.org/D61444?vs=197794&id=19780

[PATCH] D61357: SemaOverload: Complete candidates before emitting the error, to ensure diagnostics emitted (or suppressed) during completion don't interfere with the overload notes

This revision was automatically updated to reflect the committed changes. Closed by commit rC359854: SemaOverload: Complete candidates before emitting the error, to ensure… (authored by dblaikie, committed by ). Changed prior to commit: https://reviews.llvm.org/D61357?vs=197491&id=197901#toc R

[PATCH] D61357: SemaOverload: Complete candidates before emitting the error, to ensure diagnostics emitted (or suppressed) during completion don't interfere with the overload notes

dblaikie marked an inline comment as done. dblaikie added a comment. In D61357#1488839 , @rsmith wrote: > In D61357#1485581 , @dblaikie wrote: > > > Oh, @rsmith - if there's any better/different testing (if you can

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

dblaikie added inline comments. Comment at: lib/Sema/SemaType.cpp:4913-4916 +ExpectNoDerefChunk = false; + } + + ExpectNoDerefChunk = state.didParseNoDeref(); Pointed out in PR41774 there's a dead store to ExpectNoDerefChunk here. Should line 4

[PATCH] D61656: -frewrite-imports: Add support for wildcard rules in umbrella modules with

dblaikie updated this revision to Diff 198528. dblaikie added a comment. Add another FIXME Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61656/new/ https://reviews.llvm.org/D61656 Files: include/clang/Basic/Module.h lib/Basic/Module.cpp lib/Lex/Pragma.cpp

[PATCH] D61656: -frewrite-imports: Add support for wildcard rules in umbrella modules with

dblaikie created this revision. dblaikie added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. dblaikie updated this revision to Diff 198528. dblaikie added a comment. Add another FIXME This trips over a few other limitations, but in the interests of in

[PATCH] D61656: -frewrite-imports: Add support for wildcard rules in umbrella modules with

dblaikie marked an inline comment as done. dblaikie added inline comments. Comment at: test/Modules/preprocess-umbrella.cpp:1-2 +// FIXME: The standalone module still seems to cause clang to want to test for +// the existence of a 'foo' directory: +// RUN: mkdir %t --

[PATCH] D61656: -frewrite-imports: Add support for wildcard rules in umbrella modules with

This revision was automatically updated to reflect the committed changes. Closed by commit rC360195: -frewrite-imports: Add support for wildcard rules in umbrella modules with (authored by dblaikie, committed by ). Changed prior to commit: https://reviews.llvm.org/D61656?vs=198528&id=198533#toc

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Sure, looks good. Though my other/vague concern is why does this case error about fomit-frame-pointer having no effect, but other things (like using -fomit-frame-pointer on a target that r

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

dblaikie added a comment. In https://reviews.llvm.org/D52191#1238648, @srhines wrote: > In https://reviews.llvm.org/D52191#1238628, @dblaikie wrote: > > > Sure, looks good. Though my other/vague concern is why does this case error > > about fomit-frame-pointer having no effect, but other things

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

dblaikie added a subscriber: srhines. dblaikie added a comment. Fixed in r342510 with the solution I mentioned up-thread. Repository: rL LLVM https://reviews.llvm.org/D52191 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

dblaikie added a comment. Do you/what's your particular use case for this scenario? I guess this looks a bit like Apple's format (where debug info stays in the object file and isn't linked into the final binary), but don't expect they'd be moving to this any time soon. https://reviews.llvm.or

[PATCH] D52502: [Lex] TokenConcatenation now takes const Preprocessor

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. any particular motivation for this? But looks fine to me in any case. Repository: rC Clang https://reviews.llvm.org/D52502 ___ cfe-commits

[PATCH] D52315: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

dblaikie added a comment. "The three checks mentioned in the Title are two noisy if the code uses intrusive smart (reference counting) pointers. LLVM/Clang is such a code, it has lots of such types, e.g. StringRef, SymbolRef or ProgramStateRef, all of them based llvm::IntrusiveRefCntPtr. Every

[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Seems reasonable to me - but if you like you can wait for Richard who might have a more nuanced understanding of the code. (but I'm OK signing off on this if you are & Richard can provide

[PATCH] D57668: [opaque pointer types] Pass function types for runtime function calls.

dblaikie added inline comments. Comment at: clang/lib/CodeGen/CGObjC.cpp:1028-1030 +llvm::Constant *getPropertyFn = cast( +CGM.getObjCRuntime().GetPropertyGetFunction().getCallee()); if (!getPropertyFn) { This code looks like it implies that the

[PATCH] D57668: [opaque pointer types] Pass function types for runtime function calls.

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

[PATCH] D57766: [opaque pointer types] Cleanup CGBuilder's Create*GEP.

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

[PATCH] D57767: [opaque pointer types] Cleanup CGBuilder's Create*GEP.

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

[PATCH] D57801: [opaque pointer types] Pass through function types for TLS initialization and global destructor calls.

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

[PATCH] D57804: [opaque pointer types] Make EmitCall pass Function Types to CreateCall/Invoke.

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

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

dblaikie added a comment. This, I guess, is part of the impact of moving towards explicit modules (-fmodule-name is for building a module with that name, right?)? With explicit modules there is the option for modular code generation - which doesn't require any specific DWARF consumer support (s

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

dblaikie added a comment. In D57976#1393976 , @aprantl wrote: > In D57976#1393939 , @dblaikie wrote: > > > This, I guess, is part of the impact of moving towards explicit modules > > (-fmodule-name is for building

[PATCH] D57986: [ProfileData] Remove non-determinism: Change DenseMap to MapVector

dblaikie added a comment. In D57986#1398271 , @vsk wrote: > I think this could roughly double the memory utilization of the writer, which > might be problematic because the number of records to write can be high. > (@dblaikie did some work on reducing me

[PATCH] D37235: Let -Wdelete-non-virtual-dtor fire in system headers too.

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. (if you'd prefer to wait for Reid or Richard to take a look, that's OK too :) ) Looks good to me, though wouldn't mind if the tests were a bit simpler - if they can be made so.

[PATCH] D37299: [Modules] Add ability to specify module name to module file mapping in a file

dblaikie added a comment. Couple of drive by comments - no doubt Richard will have a more in depth review, but figured this might save a round or two. Comment at: lib/Frontend/CompilerInvocation.cpp:1561-1565 + if (File.empty()) + { +Diags.Report(diag::err_drv_invalid_va

[PATCH] D37428: Debug info: Fixed faulty debug locations for attributed statements

dblaikie added a comment. Not sure what you mean by "avoid emitting unnecessary stop points" - do you have a test case for that? https://reviews.llvm.org/D37428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D37428: Debug info: Fixed faulty debug locations for attributed statements

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. OK, sounds good - thanks :) https://reviews.llvm.org/D37428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D63031: DebugInfo: Render the canonical name of a class template specialization, even when nested in another class template specialization

dblaikie created this revision. dblaikie added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D63031 Files: lib/AST/TypePrinter.cpp lib/CodeGen/CGDebugInfo.cpp test/CodeGenCXX/debug-info-template-ex

[PATCH] D63130: [Clang] Rename -split-dwarf-file to -split-dwarf-output

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good - thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63130/new/ https://reviews.llvm.org/D63130 __

[PATCH] D63167: [Clang] Remove obsolete -enable-split-dwarf={single,split}

dblaikie added inline comments. Comment at: include/clang/Basic/CodeGenOptions.def:263 -ENUM_CODEGENOPT(SplitDwarfMode, DwarfFissionKind, 2, NoFission) ///< DWARF fission mode to use. +CODEGENOPT(EnableSplitDwarf, 1, 0) ///< Whether to enable split DWARF. Co

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59673/new/ https://reviews.llvm.org/D59673 ___ cfe

[PATCH] D63031: DebugInfo: Render the canonical name of a class template specialization, even when nested in another class template specialization

dblaikie marked an inline comment as done. dblaikie added a comment. Ping (just curious about the change to the canonicalization printing policy - making sure I don't break something important) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63031/new/ https://rev

[PATCH] D62635: Add enums as global variables in the IR metadata.

dblaikie added a comment. Is it practical/possible to do this in LLVM rather than in Clang? I'd have thought it'd be best to keep the IR metadata as output-format agnostic as practical/possible to reduce divergent codepaths, etc? Repository: rL LLVM CHANGES SINCE LAST ACTION https://revie

[PATCH] D62635: Add enums as global variables in the IR metadata.

dblaikie added a comment. In D62635#1547352 , @akhuang wrote: > I think the main issue was keeping track of which enums are used? The used enums would still end up in the 'enums' list for the DICompileUnit, right? (If CodeView needs more enums in the li

[PATCH] D62635: Add enums as global variables in the IR metadata.

dblaikie added a comment. In D62635#1548157 , @rnk wrote: > We did things this way to track which **enumerators** were used, not which > enums were used. Size data showed it was worth doing (6%). The existing > format doesn't support tracking usage of in

[PATCH] D62635: Add enums as global variables in the IR metadata.

dblaikie added a comment. In D62635#1548721 , @rnk wrote: > In D62635#1548648 , @dblaikie wrote: > > > In D62635#1548157 , @rnk wrote: > > > > > We did things this way to tra

[PATCH] D63031: DebugInfo: Render the canonical name of a class template specialization, even when nested in another class template specialization

dblaikie added a comment. Ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63031/new/ https://reviews.llvm.org/D63031 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D64058: [cxx2a] P0624R2 fix: only lambdas with no lambda-capture are default-constructible and assignable.

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good to me. (possible the check (for no default capture and no captures) could be unified with the same check/language used for the implicit function pointer conversion operator ("if

[PATCH] D63894: [CXX] Exercise all paths through these tests

dblaikie added a comment. I would've assumed these conditionals were added by Sony folks for their change in default dialect - that doesn't necessarily mean these tests are needed upstream (the functionality may be tested elsewhere) Repository: rC Clang CHANGES SINCE LAST ACTION https://r

[PATCH] D64058: [cxx2a] P0624R2 fix: only lambdas with no lambda-capture are default-constructible and assignable.

This revision was automatically updated to reflect the committed changes. Closed by commit rL365406: [cxx2a] P0624R2 fix: only lambdas with no lambda-capture are default… (authored by dblaikie, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D62202: Work around a Visual C++ bug

dblaikie added a comment. In D62202#1510394 , @probinson wrote: > To provide some missing details: > The original source gets a bogus compile-time error with Visual Studio 2017, > prior to version 15.8. I have 15.2, so I need this patch to build Clang.

[PATCH] D62202: Work around a Visual C++ bug

dblaikie added a comment. In D62202#1510449 , @probinson wrote: > In D62202#1510414 , @dblaikie wrote: > > > Technically this violates the LLVM style guide which says "make anonymous > > namespaces as small as poss

[PATCH] D62202: Work around a Visual C++ bug

dblaikie added a comment. In D62202#1510835 , @rnk wrote: > In D62202#1510522 , @dblaikie wrote: > > > Yeah, if we're going this way I'd certainly advocate having a comment of > > some kind explaining why it's this

[PATCH] D62202: Work around a Visual C++ bug

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. SGTM - thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62202/new/ https://reviews.llvm.org/D62202 ___

[PATCH] D62167: CodeView - add static data members to global variable debug info.

dblaikie added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4385 +// Use the global scope for static members. +DContext = getContextDescriptor( + cast(CGM.getContext().getTranslationUnitDecl()), TheCU); akhuang wrote: > @dblaikie I

[PATCH] D62167: CodeView - add static data members to global variable debug info.

dblaikie added a comment. Is this the change we were discussing the other week semi-related to preserved enums? It sounded to me like the conclusion was somewhat to go ahead without adding all these things to the constant list and see how it goes? Is this case different from the enum case in so

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

dblaikie added a comment. Might be easier as a few patches - renaming the existing option, adding the new one, then removing the single split dwarf flag handling in favor of implying that by the absence of an output file name. (if I'm reading what this patch does) Comment at

[PATCH] D55770: [clangd] BackgroundIndex rebuilds symbol index periodically.

dblaikie added a comment. (from the peanut gallery): Usually what I'd expect here is "reindexing will occur X milliseconds after a write, covering anything written up until that point" - so it doesn't reindex unnecessarily if the files are unmodified/sitting idle, and it doesn't thrash reindexi

[PATCH] D56438: [Modules] Allow modulemap path change for implicitly built modules

dblaikie added a comment. No doubt @rsmith will have a more nuanced/accurate opinion on this than I do, but I would've thought the point of implicit modules is that they don't get moved around & it sounds like that's what this patch is suggesting/supporting, but maybe I haven't understood it fu

[PATCH] D63031: DebugInfo: Render the canonical name of a class template specialization, even when nested in another class template specialization

dblaikie added a comment. Some more ping (: Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63031/new/ https://reviews.llvm.org/D63031 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D67647: [Consumed] Refactor handleCall to take function argument list. NFC.

dblaikie added inline comments. Comment at: lib/Analysis/Consumed.cpp:752 + handleCall(Call, nullptr, + llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()), + FunDecl); probably use "Call->arguments()" here & in the other places that

[PATCH] D53768: Add VerboseOutputStream to CompilerInstance

dblaikie added inline comments. Comment at: include/clang/Frontend/CompilerInstance.h:158-159 + /// Whether we should delete VerboseOutputStream on destruction. + bool OwnsVerboseOutputStream = false; + Rather than a bool, this could be a unique_ptr, perhaps?

[PATCH] D67723: [CodeView] Add option to disable inline line tables.

dblaikie added subscribers: JDevlieghere, aprantl. dblaikie added a comment. As per the bug - I'm not super inclined towards a function attribute here (& even if it's going to be an inliner rather than debug info generation/backend thing (which I'm more on the fence about - think I'm happy with

[PATCH] D67647: [Consumed] Refactor handleCall to take function argument list. NFC.

dblaikie added a comment. In D67647#1673363 , @comex wrote: > Ugh, it looks like `getArgs()` is a massive strict aliasing violation. And > it's used in enough different places in Clang that fixing the violation may > require extensive refactoring... I'v

[PATCH] D67647: [Consumed] Refactor handleCall to take function argument list. NFC.

dblaikie added a comment. In D67647#1674745 , @comex wrote: > Here's a new version of the patch that uses iterator ranges instead of > `ArrayRef`, to avoid adding new uses of `CallExpr::getArgs()` in case it has > to be removed in the future due to the s

[PATCH] D67647: [Consumed] Refactor handleCall to take function argument list. NFC.

dblaikie added a comment. In D67647#1674781 , @comex wrote: > In D67647#1674773 , @dblaikie wrote: > > > I wasn't expecting it to involve all this work - but instead to change > > "arguments()" to return ArrayRef.

[PATCH] D67743: [Consumed] Treat by-value class arguments as consuming by default, like rvalue refs.

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. "Also, fix the order of if statements so that an explicit return_typestate annotation takes precedence over the default behavior for rvalue refs." I'd probably have split that out into a s

[PATCH] D67740: [Consumed] Refactor and improve diagnostics

dblaikie added a comment. Looks like this might benefit from being split into independent changes - the work related to templates (I haven't looked closely, but I assume that's fairly indivisible) and the work related to other diagnostics seems fairly separable - and maybe there's other pieces

[PATCH] D67778: [Consumed] Narrow Subject for some attributes

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good - thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67778/new/ https://reviews.llvm.org/D67778 __

[PATCH] D67647: [Consumed] Refactor handleCall to take function argument list. NFC.

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Sounds good - thanks! Comment at: llvm/include/llvm/ADT/iterator_range.h:27-33 +template +constexpr bool is_random_iterator() { + return std::is_same< +typename std

[PATCH] D67647: [Consumed] Refactor handleCall to take function argument list. NFC.

dblaikie added inline comments. Comment at: llvm/include/llvm/ADT/iterator_range.h:27-33 +template +constexpr bool is_random_iterator() { + return std::is_same< +typename std::iterator_traits::iterator_category, +std::random_access_iterator_tag>::value; +} + ---

[PATCH] D67740: [Consumed] Refactor and improve diagnostics

dblaikie accepted this revision. dblaikie added inline comments. This revision is now accepted and ready to land. Comment at: lib/Sema/SemaDeclAttr.cpp:1249 + Diag(A->getLoc(), diag::warn_consumable_attr_for_unconsumable_type) << +A << (isa(D) ? 0 : 1) << Type; + return fal

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

dblaikie added a comment. +1 for clang-tidy. GCC's closest to this is -Weffc++ which limits the rule of 3 warning to only "classes that have dynamic memory allocation" (though I'm not sure exactly what conditions it uses to decide that - it doesn't warn on anything in the test cases provided in

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

dblaikie added a subscriber: lebedev.ri. dblaikie added a comment. I'm not sure this is a practical direction to pursue - though perhaps others disagree. It's likely non-trivial to plumb a flag through most build systems to be applied only to test code (& likely would suppress the warning in head

[PATCH] D41357: WIP: Fix Diagnostic layering, moving diagnostics out of Basic

dblaikie added a comment. I've pushed this further - it builds now & has no fallback in libBasic to the old behavior/table. One remaining thing I'm unsure about is the table of DIAG_START_* values. Do you think this is OK/good to leave hardcoded in diagnostics? Should I somehow remove the need

[PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

dblaikie added inline comments. Comment at: lib/Sema/SemaExpr.cpp:11527-11528 - S.Diag(OpLoc, diag::warn_self_assignment) - << LHSDeclRef->getType() - << LHSExpr->getSourceRange() << RHSExpr->getSourceRange(); + S.Diag(OpLoc, IsBuiltin ? diag::warn_self_assignment_b

[PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

dblaikie accepted this revision. dblaikie added inline comments. This revision is now accepted and ready to land. Comment at: lib/Sema/SemaExpr.cpp:11527-11528 - S.Diag(OpLoc, diag::warn_self_assignment) - << LHSDeclRef->getType() - << LHSExpr->getSourceRange() << RH

[PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

dblaikie added a subscriber: lebedev.ri. dblaikie added a comment. FWIW I don't fundamentalyl object to also having something like -wtest. Probably needs a better name though (unfortunately the double-negative gets confusing... - like you want to describe the set of diagnostics that should not be

[PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

dblaikie added a comment. Is there anything else in the "-w" namespace other than the literal "-w" so far? I mean, I could imagine it might make more sense to default these warnings off & users can turn them on for non-test code, potentially? So "-Wnon-test" might make sense. Repository: rL L

[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct

dblaikie added a comment. This doesn't seem to build for me - so hard to debug/probe it: llvm/src/tools/clang/lib/CodeGen/CGBuiltin.cpp:1264:65: error: no viable conversion from 'clang::QualType' to 'llvm::Type *' CGF.CGM.getDataLayout().getTypeSizeInBits(CanonicalType),

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

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

[PATCH] D48571: improve diagnostics for missing 'template' keyword

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks pretty good to me - nice work! Repository: rL LLVM https://reviews.llvm.org/D48571 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct

dblaikie added a subscriber: paulsemel. dblaikie added a comment. Yeah, doesn't look like this code handles a value crossing the boundary of the size of the bitfield type (it's reading only the low bit). I suspect looking at the code that generates bitfield accesses would be useful - and/or maybe

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

dblaikie added a comment. Any chance this can/should be unit tested? (also, in general (though might not matter in this instance), symmetric operators like == should be implemented as non-members (though they can still be friends and if they are, can be defined inline in the class definition as a

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. In theory you'd need separate tests for op== and op!= returning false (currently all the tests would pass if both implementations returned true in all cases), but not the biggest deal. R

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

dblaikie accepted this revision. dblaikie added a comment. Looks good, Thanks! Repository: rC Clang https://reviews.llvm.org/D49265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68410: [AttrDocs] document always_inline

dblaikie added a comment. > In D68410#1696411 , @joerg wrote: > >> I wonder if we should actually enumerate evil here, i.e. give the situations >> in which inlining actually fails. > > > Which is likely to change over time. I worry that enumerating suc

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

dblaikie added a comment. In D68117#1707578 , @SouraVX wrote: > In D68117#1702595 , @probinson wrote: > > > We really do want to pack the four mutually exclusive cases into two bits. > > I have tried to give more

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

dblaikie added a comment. In D68117#1708114 , @SouraVX wrote: > In D68117#1707680 , @dblaikie wrote: > > > In D68117#1707578 , @SouraVX wrote: > > > > > In D68117#1702595

[PATCH] D67723: [DebugInfo] Add option to disable inline line tables.

dblaikie added a comment. In D67723#1708671 , @aprantl wrote: > > Since that change, we treat line zero the same as "no location". If there > > are no locations in a basic block, then the whole block inherits the line > > number from the block layout pre

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

dblaikie added a comment. In D68117#1709712 , @probinson wrote: > In D68117#1709557 , @SouraVX wrote: > > > Their's not much information available behind the suggestion or intention > > for adding this feature to S

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

dblaikie added a comment. In D68117#1711154 , @probinson wrote: > In D68117#1710295 , @dblaikie wrote: > > > I think the existing feature's perhaps a bit misspecified - because where > > you put the DEFAULTED_{in_c

[PATCH] D67647: [Consumed] Refactor handleCall to take function argument list. NFC.

dblaikie added a comment. In D67647#1713974 , @comex wrote: > So, I landed this patch but had to revert it as it broke the build on MSVC > (and only MSVC). That was almost a month ago, but I haven't gotten back > around to this until now – sorry :| > >

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

dblaikie added a subscriber: rsmith. dblaikie added a comment. In D68117#1714091 , @SouraVX wrote: > Hi @probinson @dblaikie @aprantl , I've was investigating and working on > your inputs regarding the problem with DW_at_defaulted once. I think clang >

[PATCH] D63031: DebugInfo: Render the canonical name of a class template specialization, even when nested in another class template specialization

This revision was automatically updated to reflect the committed changes. Closed by commit rG9fdd09a4ccd0: DebugInfo: Render the canonical name of a class template specialization, even… (authored by dblaikie). Changed prior to commit: https://reviews.llvm.org/D63031?vs=203639&id=225718#toc Rep

[PATCH] D67723: [DebugInfo] Add option to disable inline line tables.

dblaikie added a comment. In D67723#1720509 , @aprantl wrote: > In D67723#1720353 , @rnk wrote: > > > In D67723#1717468 , @aprantl wrote: > > > > > I agree that it would make

[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

dblaikie added a comment. Looks like this only implements support for typedefs, but the flag in GCC does more than that (as the flag name indicates - it's about unused types in general) - could you test this across some non-trivial code and see if it matches GCC's behavior (or, where it doesn't

[PATCH] D79967: Fix debug info for NoDebug attr

dblaikie added a comment. In D79967#2045196 , @vsk wrote: > @yaxunl thanks, this patch lgtm. > > @dblaikie I've kicked off a thread on cfe-dev about the topics you brought up > ("Design discussion re: DW_TAG_call_site support in clang") and cc'd you. Th

[PATCH] D80391: [Driver] Don't make -gsplit-dwarf imply -g2

dblaikie added a comment. I think it's probably best to keep this subject on the cfe-dev thread until there's agreement there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80391/new/ https://reviews.llvm.org/D80391

[PATCH] D80369: WIP: [DebugInfo] Remove decl subprograms from 'retainedTypes:'

dblaikie added a comment. In D80369#2048932 , @djtodoro wrote: > Still have test failing: > > Clang :: Modules/DebugInfoTransitiveImport.m > Clang :: Modules/ModuleDebugInfo.cpp > Clang :: Modules/ModuleDebugInfo.m > > > I haven't looked into the

[PATCH] D80369: [DebugInfo][CallSites] Remove decl subprograms from 'retainedTypes:'

dblaikie added a comment. In D80369#2050962 , @djtodoro wrote: > In D80369#2050022 , @dblaikie wrote: > > > In D80369#2048932 , @djtodoro > > wrote: > > > > > Still have tes

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