Page MenuHomePhabricator

dexonsmith (Duncan P. N. Exon Smith)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 18 2014, 10:33 AM (356 w, 5 d)

Recent Activity

Yesterday

dexonsmith added a comment to D94739: ADT: Fix reference invalidation in SmallVector::emplace_back and the size+value version of SmallVector::assign.

[...] maybe I can trigger [the] compile-time tracker myself [...]

Sun, Jan 17, 3:09 PM · Restricted Project
dexonsmith accepted D94857: [VFS] Combine VFSFromYamlDirIterImpl and OverlayFSDirIterImpl into a single implementation (NFC).

LGTM! Thanks for splitting it out.

Sun, Jan 17, 2:52 PM · Restricted Project

Fri, Jan 15

dexonsmith added a reviewer for D94712: Do not traverse ConstantData use-list in LookInterchange: fhahn.

@fhahn, can you help with the review here? This is one of only a few instances of walking the use-list of ConstantData. The broader context is an old RFC that @willir is working with me on:
https://lists.llvm.org/pipermail/llvm-dev/2016-September/105157.html

Fri, Jan 15, 5:36 PM · Restricted Project
dexonsmith added reviewers for D94713: Do not traverse ConstantData use-list in SLPVectorizer: nikic, spatel, bjope, anton-afanasyev.

Adding some reviewers that have touched the SLPVectorizer recently to check this. (I have context on the motivation for the change and planned changes to the IR, but I don't know this pass well.)

Fri, Jan 15, 5:23 PM · Restricted Project
dexonsmith added a reviewer for D94844: [VFS] Add support to RedirectingFileSystem for mapping a virtual directory to one in the external FS.: dexonsmith.

Yeah, certainly. Let me put that up separately and update this diff.

Fri, Jan 15, 4:58 PM · Restricted Project, Restricted Project
dexonsmith added a comment to D94844: [VFS] Add support to RedirectingFileSystem for mapping a virtual directory to one in the external FS..

This patch also refactors OverlayFileSystem's directory iterator implementation (OverlayFSDirIterImpl) and VFSFromYamlDirIterImpl into a single implementation, addressing a FIXME about their conceptual similarity.

Fri, Jan 15, 4:53 PM · Restricted Project, Restricted Project
dexonsmith added a comment to D94811: [lldb] Fix fallthrough with strictly virtual working directory..

Personally I'd just like to get rid of ExternalFSValidWD inside of shouldUseExternalFS and let the underlying FS take care of it, which I believe would better fit the abstraction but would be a change in behavior (as pointed out in https://reviews.llvm.org/D65677#inline-604694).

Fri, Jan 15, 4:44 PM · Restricted Project
dexonsmith updated the diff for D94739: ADT: Fix reference invalidation in SmallVector::emplace_back and the size+value version of SmallVector::assign.

Remove some unnecessary templates in SmallVector.cpp. I just realized I hadn't uploaded this version of the patch, which is what I ran the perf on.

Fri, Jan 15, 3:43 PM · Restricted Project
dexonsmith added a reviewer for D94739: ADT: Fix reference invalidation in SmallVector::emplace_back and the size+value version of SmallVector::assign: nikic.

I did a spot check of code size / compile-time, capturing clang's binary size in KiB (78460KiB => 78484KiB) and 5 runs of compiling VirtualFileSystem.cpp.o (looks like it's in the noise):

78460	baseline/build/bin/clang
        3.84 real         3.74 user         0.09 sys
        3.85 real         3.75 user         0.09 sys
        3.83 real         3.73 user         0.09 sys
        3.84 real         3.73 user         0.10 sys
        3.84 real         3.74 user         0.09 sys
78484	invalidation/build/bin/clang
        3.83 real         3.73 user         0.09 sys
        3.87 real         3.77 user         0.09 sys
        3.83 real         3.73 user         0.09 sys
        3.83 real         3.72 user         0.10 sys
        3.90 real         3.80 user         0.09 sys

"baseline" is ceaf0110ff5e0c2de1f03d65d13703d34d0d5737, and "invalidation" has this patch applied on top.

Fri, Jan 15, 3:41 PM · Restricted Project
dexonsmith added a reverting change for rG33be50daa9ce: Revert "Reapply "ADT: Fix reference invalidation in SmallVector::push_back and…: rGceaf0110ff5e: Revert "Revert "ADT: Fix reference invalidation in SmallVector..."".
Fri, Jan 15, 2:30 PM
dexonsmith committed rGceaf0110ff5e: Revert "Revert "ADT: Fix reference invalidation in SmallVector..."" (authored by dexonsmith).
Revert "Revert "ADT: Fix reference invalidation in SmallVector...""
Fri, Jan 15, 2:30 PM
dexonsmith closed D94800: ADT: Skip SmallVector::isReferenceToStorage when TakesParamByValue, NFC.
Fri, Jan 15, 2:30 PM · Restricted Project
dexonsmith added a comment to D94800: ADT: Skip SmallVector::isReferenceToStorage when TakesParamByValue, NFC.

I tested the original three patches plus this one, and the results look good (or at least in line with expectations): https://llvm-compile-time-tracker.com/compare.php?from=33be50daa9ce1074c3b423a4ab27c70c0722113a&to=972a64b1331d1b3022154297593837234a958d0b&stat=instructions

Fri, Jan 15, 1:51 PM · Restricted Project
dexonsmith added a comment to D94811: [lldb] Fix fallthrough with strictly virtual working directory..

I missed a few words:

What if someone does setWorkingDirectory() for /a/b/c/d.c followed by

"changing directory to"

/x, and then looks up the relative path d.c? Or changes directory to ../c?

Fri, Jan 15, 1:25 PM · Restricted Project
dexonsmith added a comment to D94811: [lldb] Fix fallthrough with strictly virtual working directory..

I'm wondering about this:

Currently, calling setCurrentWorkingDirectory on RedirectingFileSystem
will attempt to change the working directory for the external FS and if
that fails, it will set ExternalFSValidWD to false which prevents
fallthrough.

Fri, Jan 15, 1:23 PM · Restricted Project
dexonsmith added a comment to D94472: [WIP][clang][cli] Command line round-trip for HeaderSearch options.

A concern I have is that the round-tripping might be too low level since it requires each group of options to opt-in somehow. Having something at the top-level that doesn't have to be touched again would be easier to validate / understand, and might better serve the purpose of verifying that the lower-level details were implemented correctly by not depending on them to turn it on.

Fri, Jan 15, 1:03 PM · Restricted Project, Restricted Project
dexonsmith added a comment to D94739: ADT: Fix reference invalidation in SmallVector::emplace_back and the size+value version of SmallVector::assign.

Note that this depends on https://reviews.llvm.org/D93781 and predecessors, which were reverted due to the lack of https://reviews.llvm.org/D94800.

Fri, Jan 15, 10:14 AM · Restricted Project
dexonsmith added a comment to D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.

@xbolva00, thanks for reporting the regression. After the performance analysis was finished in https://reviews.llvm.org/D91837, I split the patch up for more detailed review, and in responding to review comments I somehow lost the critical check against TakesParamByValue.

Fri, Jan 15, 10:11 AM · Restricted Project
dexonsmith added a reviewer for D94800: ADT: Skip SmallVector::isReferenceToStorage when TakesParamByValue, NFC: xbolva00.

@xbolva00, adding you here as well, since you reported the regression in https://reviews.llvm.org/D93779.

Fri, Jan 15, 9:53 AM · Restricted Project
dexonsmith added a reviewer for D94800: ADT: Skip SmallVector::isReferenceToStorage when TakesParamByValue, NFC: nikic.

Actually, I'm going to commit this immediately and not wait for review. Having caught up on my email that I missed yesterday, there's an active compile-time regression that this should fix. See https://reviews.llvm.org/D93779 for details.

(Happy to respond to comments post-commit of course.)

Fri, Jan 15, 9:50 AM · Restricted Project
dexonsmith added a comment to D94800: ADT: Skip SmallVector::isReferenceToStorage when TakesParamByValue, NFC.

Actually, I'm going to commit this immediately and not wait for review. Having caught up on my email that I missed yesterday, there's an active compile-time regression that this should fix. See https://reviews.llvm.org/D93779 for details.

Fri, Jan 15, 9:43 AM · Restricted Project
dexonsmith added a comment to D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.

Over in D91837 the reserveForAndGetAddress() implementation contained a check for TakesParamByValue that seems to be gone in this revision. My first guess is that this is the culprit, and we're now always performing the isReferenceToStorage() check.

Fri, Jan 15, 9:41 AM · Restricted Project
dexonsmith updated the diff for D94739: ADT: Fix reference invalidation in SmallVector::emplace_back and the size+value version of SmallVector::assign.

Simplified emplace_back and addressed the concern I had with copying large POD-like types too often.

  • Moved it back to SmallVectorImpl, only delegating to SmallVectorTemplateBase for growAndEmplaceBack().
  • Now it only defers to push_back when growing (still just for POD-like types).
  • Also improved the comment to explain why the copy makes sense even for large POD-like types: that it's better than losing the realloc optimization.
Fri, Jan 15, 9:34 AM · Restricted Project
dexonsmith requested review of D94800: ADT: Skip SmallVector::isReferenceToStorage when TakesParamByValue, NFC.
Fri, Jan 15, 9:25 AM · Restricted Project
dexonsmith planned changes to D94739: ADT: Fix reference invalidation in SmallVector::emplace_back and the size+value version of SmallVector::assign.

Note that this is a follow-up to https://reviews.llvm.org/D91837 (and the patches split out from it), and an alternative implementation for https://reviews.llvm.org/D87326.

Fri, Jan 15, 8:18 AM · Restricted Project

Thu, Jan 14

dexonsmith requested review of D94739: ADT: Fix reference invalidation in SmallVector::emplace_back and the size+value version of SmallVector::assign.
Thu, Jan 14, 6:42 PM · Restricted Project

Wed, Jan 13

dexonsmith committed rG6ed3083a9654: ADT: Reduce code duplication in SmallVector by calling reserve and clear, NFC (authored by dexonsmith).
ADT: Reduce code duplication in SmallVector by calling reserve and clear, NFC
Wed, Jan 13, 9:11 PM
dexonsmith committed rG3f98b66f23f9: ADT: Reduce code duplication in SmallVector by reusing reserve, NFC (authored by dexonsmith).
ADT: Reduce code duplication in SmallVector by reusing reserve, NFC
Wed, Jan 13, 8:54 PM
dexonsmith committed rGc224a834583c: ADT: Reduce code duplication in SmallVector::resize by using pop_back_n, NFC (authored by dexonsmith).
ADT: Reduce code duplication in SmallVector::resize by using pop_back_n, NFC
Wed, Jan 13, 8:53 PM
dexonsmith committed rG260a856c2abc: ADT: Fix reference invalidation in SmallVector::resize (authored by dexonsmith).
ADT: Fix reference invalidation in SmallVector::resize
Wed, Jan 13, 8:48 PM
dexonsmith closed D93781: ADT: Fix reference invalidation in SmallVector::resize.
Wed, Jan 13, 8:48 PM · Restricted Project
dexonsmith added a comment to D93781: ADT: Fix reference invalidation in SmallVector::resize.

Looks good (did you happen to try this with sanitizers without the fix applied & observe a sanitizer failure when running the updated test? Would be nice to know that's actually working as intended)

Wed, Jan 13, 8:46 PM · Restricted Project
dexonsmith updated subscribers of D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.

This patch seems to have found a bug: in AArch64InstrInfo.cpp, there is a -Wsometimes-uninitialized warning that now triggers, and adding an assertion to make sure the value is eventually set causes a few tests to fail. The usage is DelInstrs.push_back(MUL), which I guess is how it ties in to this patch.

More details in 752fafda3dbf1f4885c64408a13ddb67c91ccb13, which gets things back to building, but doesn't do anything about the bug.

Oh weird, I saw that too, but (wrongly) assumed it was unrelated to my change. Thanks for digging into it.

Wed, Jan 13, 8:44 PM · Restricted Project
dexonsmith added a comment to D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.

This patch seems to have found a bug: in AArch64InstrInfo.cpp, there is a -Wsometimes-uninitialized warning that now triggers, and adding an assertion to make sure the value is eventually set causes a few tests to fail. The usage is DelInstrs.push_back(MUL), which I guess is how it ties in to this patch.

More details in 752fafda3dbf1f4885c64408a13ddb67c91ccb13, which gets things back to building, but doesn't do anything about the bug.

Wed, Jan 13, 8:39 PM · Restricted Project
dexonsmith added a comment to D93780: ADT: Fix reference invalidation in N-element SmallVector::append and insert.

Thanks for the review! Landed in 3043e5a5c33c4c871f4a1dfd621a8839f9a1f0b3.

Wed, Jan 13, 8:25 PM · Restricted Project
dexonsmith committed rG3043e5a5c33c: ADT: Fix reference invalidation in N-element SmallVector::append and insert (authored by dexonsmith).
ADT: Fix reference invalidation in N-element SmallVector::append and insert
Wed, Jan 13, 8:20 PM
dexonsmith closed D93780: ADT: Fix reference invalidation in N-element SmallVector::append and insert.
Wed, Jan 13, 8:20 PM · Restricted Project
dexonsmith added a reverting change for rG56d1ffb927d0: Revert "ADT: Fix reference invalidation in SmallVector::push_back and single…: rG49142991a685: Reapply "ADT: Fix reference invalidation in SmallVector::push_back and single….
Wed, Jan 13, 7:46 PM
dexonsmith committed rG49142991a685: Reapply "ADT: Fix reference invalidation in SmallVector::push_back and single… (authored by dexonsmith).
Reapply "ADT: Fix reference invalidation in SmallVector::push_back and single…
Wed, Jan 13, 7:46 PM
dexonsmith closed D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.
Wed, Jan 13, 7:45 PM · Restricted Project
dexonsmith added a comment to D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.

Reverted in 56d1ffb927d03958a7a31442596df749264a7792 due to an error on Windows:
http://lab.llvm.org:8011/#/builders/127/builds/4489/steps/7/logs/stdio
if it's obvious I'll recommit in a minute, but if anyone has ideas I'd welcome them.

Guess maybe something in the std::is_same<std::remove_const_t<std::remove_reference_t<ArgType>>, T>::value isn't working quite right on MSVC? (could test that with godbolt, maybe) - is it needed? Since it's on both insert_one_maybe_copy and there's an implementation detail/doesn't need to protect from violations of that constraint (admittedly I'm proposing removing it because maybe MSVC is violating it... )? And instead SFINAE/enable_if only on the TakesParamByValue part?

Wed, Jan 13, 7:33 PM · Restricted Project
dexonsmith updated the diff for D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.

Replacing insert_one_maybe_copy indirection with a forwarding helper called forward_value_param.

Wed, Jan 13, 7:25 PM · Restricted Project
dexonsmith reopened D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.

Reverted in 56d1ffb927d03958a7a31442596df749264a7792 due to an error on Windows:
http://lab.llvm.org:8011/#/builders/127/builds/4489/steps/7/logs/stdio
if it's obvious I'll recommit in a minute, but if anyone has ideas I'd welcome them.

Wed, Jan 13, 7:05 PM · Restricted Project
dexonsmith added a reverting change for rG9abac6030900: ADT: Fix reference invalidation in SmallVector::push_back and single-element…: rG56d1ffb927d0: Revert "ADT: Fix reference invalidation in SmallVector::push_back and single….
Wed, Jan 13, 7:04 PM
dexonsmith committed rG56d1ffb927d0: Revert "ADT: Fix reference invalidation in SmallVector::push_back and single… (authored by dexonsmith).
Revert "ADT: Fix reference invalidation in SmallVector::push_back and single…
Wed, Jan 13, 7:04 PM
dexonsmith added a reverting change for D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert: rG56d1ffb927d0: Revert "ADT: Fix reference invalidation in SmallVector::push_back and single….
Wed, Jan 13, 7:04 PM · Restricted Project
dexonsmith committed rG9abac6030900: ADT: Fix reference invalidation in SmallVector::push_back and single-element… (authored by dexonsmith).
ADT: Fix reference invalidation in SmallVector::push_back and single-element…
Wed, Jan 13, 6:59 PM
dexonsmith closed D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.
Wed, Jan 13, 6:59 PM · Restricted Project

Tue, Jan 12

dexonsmith added a comment to D93953: NFC: Remove simple_ilist comment mentioning ilist/iplist allocating.

On a similar note, I noticed elsewhere after fixing this that you mentioned in a commit message that you intended to remove iplist and ilist and rename as owning_ilist (or something of that sorts). Is that still on the table?

Tue, Jan 12, 4:49 PM · Restricted Project
dexonsmith added inline comments to D94486: [LTO] Expose opt() in LTOBackend (NFC)..
Tue, Jan 12, 4:38 PM · Restricted Project
dexonsmith added a comment to D94472: [WIP][clang][cli] Command line round-trip for HeaderSearch options.

It'd be much nicer to add the tests as part of the patch. I've added a comment to D94474 with a possible way to do that.

Tue, Jan 12, 10:39 AM · Restricted Project, Restricted Project
dexonsmith added a comment to D94474: [WIP][clang][cli] Test manual header search argument generation with round-trip.

I'm wondering if we can get this in incrementally without needing to list in code which options are correctly handled. Here's one way to do it:

  • Add a tool, clang-cc1-args, that shows the diff between round-tripping when given a set of args. (This could potentially grow other functionality vs. diffing. So maybe clang-cc1-args diff [args] would be the usage?)
  • Add RUN lines to tests that test the args we think should round-trip correctly and confirm there's no diff. This can be done incrementally as options get handled.
  • Only once we think we have all the options covered, add the assertion from this patch.
Tue, Jan 12, 10:39 AM · Restricted Project, Restricted Project
dexonsmith added inline comments to D94474: [WIP][clang][cli] Test manual header search argument generation with round-trip.
Tue, Jan 12, 10:29 AM · Restricted Project, Restricted Project
dexonsmith accepted D94488: [clang][cli] Port more CodeGenOptions to marshalling infrastructure.

LGTM, with one nit. It'd be nice to add more detail to the commit message as well.

Tue, Jan 12, 10:21 AM · Restricted Project
dexonsmith accepted D94499: [libc++] Rename check-cxx-deps to cxx-test-depends for consistency.

Can / should this be added to the catch-all test-depends as well? (I thought llvm-test-depends and clang-test-depends were both added there somewhere, although I admit I just the code and I don't see it...) Regardless, this could be done separately / later if it makes sense to do.

Tue, Jan 12, 10:05 AM · Restricted Project

Mon, Jan 11

dexonsmith added a comment to D93777: ADT: Fix pointer comparison UB in SmallVector.

Thanks for the review! Committed with your suggestions in 5ccff5aaa68ab789834c4463ce05b05e57593b34.

Mon, Jan 11, 3:31 PM · Restricted Project
dexonsmith committed rG5ccff5aaa68a: ADT: Fix pointer comparison UB in SmallVector (authored by dexonsmith).
ADT: Fix pointer comparison UB in SmallVector
Mon, Jan 11, 3:31 PM
dexonsmith closed D93777: ADT: Fix pointer comparison UB in SmallVector.
Mon, Jan 11, 3:31 PM · Restricted Project
dexonsmith added a comment to D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.

Thanks for the review! (Just coming back to this now after the holidays.)

Mon, Jan 11, 1:11 PM · Restricted Project
dexonsmith updated the diff for D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.

Rebase and respond to review feedback.

Mon, Jan 11, 1:01 PM · Restricted Project

Fri, Jan 8

dexonsmith added a comment to rG9a3f892d0182: [Signal] Allow one-shot SIGPIPE handler to be reached.

FWIW, that logic look corrects to me.

Fri, Jan 8, 10:01 AM

Thu, Jan 7

dexonsmith accepted D84673: [clang][cli] Port DiagnosticOpts to new option parsing system.

LGTM.

Thu, Jan 7, 11:56 AM · Restricted Project, Restricted Project

Wed, Jan 6

dexonsmith added a comment to D93395: [clang][cli] Remove -f[no-]trapping-math from -cc1 command line.

Please also update the commit message to explain why this is safe (that -fp-exception-behavior fully encapsulates the semantics for -cc1) rather than just saying the options were ignored (which sounds like a bug).

Wed, Jan 6, 1:20 PM · Restricted Project
dexonsmith accepted D93395: [clang][cli] Remove -f[no-]trapping-math from -cc1 command line.

LGTM. I've just done a careful audit myself, and I'm now confident this patch is correct and that there is no latent bug -- that it's correct to ignore -f*trapping-math on the -cc1 command-line since -fp-exception-mode will always have a value matching / superseding -f*trapping-math.

Wed, Jan 6, 1:16 PM · Restricted Project
dexonsmith requested changes to D92160: [clang] Fix wrong FDs are used for files with same name in Tooling.

(Marking as "Request Changes" to drop this from my queue; feel free to reach out if the direction I suggested isn't working well...)

Wed, Jan 6, 12:51 PM · Restricted Project
dexonsmith accepted D93701: [clang][cli] NFC: Ensure non-null DiagnosticsEngine in ParseDiagnosticArgs.
Wed, Jan 6, 12:43 PM · Restricted Project
dexonsmith added a comment to D93701: [clang][cli] NFC: Ensure non-null DiagnosticsEngine in ParseDiagnosticArgs.

With the above adjustment, this LGTM.

Wed, Jan 6, 12:43 PM · Restricted Project
dexonsmith added inline comments to D93701: [clang][cli] NFC: Ensure non-null DiagnosticsEngine in ParseDiagnosticArgs.
Wed, Jan 6, 12:41 PM · Restricted Project
dexonsmith accepted D93702: [clang][cli] NFC: Make parsing macro reusable.

LGTM.

Wed, Jan 6, 12:36 PM · Restricted Project
dexonsmith added a comment to D93981: Fix some compiler warnings.

For Wpessimizing-move : is this a clang or gcc warning? I remember that they would give opposite interpretation in this case.

Wow, I'd have expected this to be decided by the C++ standard. This is from GCC I believe.

Wed, Jan 6, 12:35 PM · Restricted Project, Restricted Project
dexonsmith accepted D94172: [clang][cli] NFC: Move parseSimpleArgs.

LGTM!

Wed, Jan 6, 12:24 PM · Restricted Project
dexonsmith added inline comments to D84673: [clang][cli] Port DiagnosticOpts to new option parsing system.
Wed, Jan 6, 12:22 PM · Restricted Project, Restricted Project

Tue, Jan 5

dexonsmith added a comment to D78058: option to write files to memory instead of disk.

@dexonsmith thanks for sharing!
Some initial thoughts since abstracting outputs is something we're starting to care about too...

Thanks for looking.

Tue, Jan 5, 5:55 PM · Restricted Project
dexonsmith added a comment to D78058: option to write files to memory instead of disk.

@dexonsmith thanks for sharing!
Some initial thoughts since abstracting outputs is something we're starting to care about too...

Tue, Jan 5, 1:09 PM · Restricted Project
dexonsmith accepted D93953: NFC: Remove simple_ilist comment mentioning ilist/iplist allocating.

LGTM. Thanks! (Please reference the commit that removed allocation in the commit message.)

Tue, Jan 5, 12:35 PM · Restricted Project
dexonsmith added inline comments to D84673: [clang][cli] Port DiagnosticOpts to new option parsing system.
Tue, Jan 5, 12:10 PM · Restricted Project, Restricted Project
dexonsmith added inline comments to D93702: [clang][cli] NFC: Make parsing macro reusable.
Tue, Jan 5, 11:50 AM · Restricted Project
dexonsmith added a comment to D93701: [clang][cli] NFC: Ensure non-null DiagnosticsEngine in ParseDiagnosticArgs.

This will make more sense after looking at D84673. In short: parsing of diagnostic options can happen outside of CompilerInvocation::createFromArgs, that's why ParseDiagnosticArgs is a free function. Ideally, we'd like to reuse the existing marshalling infrastructure here as well.

ParseDiagnosticArgs is usually called with DiagnosticEngine *Diags = nullptr. That's because the call happens early on during compiler execution, where we don't have a diagnostic engine yet. This call needs to happen first, to obtain the diagnostic options that are necessary for the engine instantiation.

This patch accommodates this free function by turning all DiagnosticEngine references to pointers. Another solution would be to create a "dummy" DiagnosticsEngine for the ParseDiagnosticArgs calls, but I didn't find a way to do that.

Tue, Jan 5, 11:39 AM · Restricted Project
dexonsmith added a comment to D93701: [clang][cli] NFC: Ensure non-null DiagnosticsEngine in ParseDiagnosticArgs.

Another solution would be to create a "dummy" DiagnosticsEngine for the ParseDiagnosticArgs calls, but I didn't find a way to do that.

Something like CompilerInstance::createDiagnostics(new DiagnosticOptions(), new IgnoreDiagnostics()) could work. I'll give that a try.

Tue, Jan 5, 11:30 AM · Restricted Project

Wed, Dec 23

dexonsmith added a comment to D78058: option to write files to memory instead of disk.

I'll reply here once I've posted the RFC and patch (as I said, I'm hoping next week) so you can take a look.

Wed, Dec 23, 8:19 PM · Restricted Project
dexonsmith committed rG3ee43adfb20d: Basic: Add native support for stdin to SourceManager and FileManager (authored by dexonsmith).
Basic: Add native support for stdin to SourceManager and FileManager
Wed, Dec 23, 3:33 PM
dexonsmith closed D93148: Basic: Add native support for stdin to SourceManager and FileManager.
Wed, Dec 23, 3:33 PM · Restricted Project
dexonsmith committed rG245218bb3555: Basic: Support named pipes natively in SourceManager and FileManager (authored by dexonsmith).
Basic: Support named pipes natively in SourceManager and FileManager
Wed, Dec 23, 2:59 PM
dexonsmith closed D92531: Basic: Support named pipes natively in SourceManager.
Wed, Dec 23, 2:58 PM · Restricted Project
dexonsmith abandoned D91467: ADT: Take small enough, trivially copyable T by value in SmallVector.

Abandoning this in favour of:

Wed, Dec 23, 2:43 PM · Restricted Project
dexonsmith abandoned D91837: ADT: Fix reference invalidation in SmallVector APIs that pass in a value.

Thanks again for the reviews here. For landing this (and a bit more review from @dblaikie) I've split this up into:

Wed, Dec 23, 2:40 PM · Restricted Project
dexonsmith added inline comments to D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.
Wed, Dec 23, 2:24 PM · Restricted Project
dexonsmith added inline comments to D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.
Wed, Dec 23, 2:18 PM · Restricted Project
dexonsmith requested review of D93781: ADT: Fix reference invalidation in SmallVector::resize.
Wed, Dec 23, 2:05 PM · Restricted Project
dexonsmith requested review of D93780: ADT: Fix reference invalidation in N-element SmallVector::append and insert.
Wed, Dec 23, 2:02 PM · Restricted Project
dexonsmith requested review of D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert.
Wed, Dec 23, 1:54 PM · Restricted Project
dexonsmith added inline comments to D93777: ADT: Fix pointer comparison UB in SmallVector.
Wed, Dec 23, 1:26 PM · Restricted Project
dexonsmith requested review of D93777: ADT: Fix pointer comparison UB in SmallVector.
Wed, Dec 23, 1:24 PM · Restricted Project

Tue, Dec 22

dexonsmith added inline comments to D84673: [clang][cli] Port DiagnosticOpts to new option parsing system.
Tue, Dec 22, 11:22 PM · Restricted Project, Restricted Project
dexonsmith added inline comments to D93702: [clang][cli] NFC: Make parsing macro reusable.
Tue, Dec 22, 11:18 PM · Restricted Project
dexonsmith added a comment to D93701: [clang][cli] NFC: Ensure non-null DiagnosticsEngine in ParseDiagnosticArgs.

This is needed for a future patch, where we start using normalizers in contexts where no Diags are available.

Tue, Dec 22, 11:15 PM · Restricted Project
dexonsmith accepted D93679: [clang][cli] Port getAllArgumentValues to the marshalling infrastructure.

LGTM.

Tue, Dec 22, 11:07 PM · Restricted Project
dexonsmith accepted D93698: [clang][cli] Port a CommaJoined option to the marshalling infrastructure.

LGTM, with one nit.

Tue, Dec 22, 11:05 PM · Restricted Project
dexonsmith added a comment to D91837: ADT: Fix reference invalidation in SmallVector APIs that pass in a value.

Thanks for the review!

Tue, Dec 22, 6:52 PM · Restricted Project

Mon, Dec 21

dexonsmith added a comment to D91837: ADT: Fix reference invalidation in SmallVector APIs that pass in a value.

Great, thanks for doing that analysis @nikic!

Mon, Dec 21, 3:24 PM · Restricted Project