Page MenuHomePhabricator

njames93 (Nathan James)
Dodd

Projects

User does not belong to any projects.

User Details

User Since
Dec 23 2019, 11:05 AM (56 w, 5 d)

Recent Activity

Fri, Jan 22

njames93 updated the diff for D94942: [clangd] Add tweak for implementing abstract class.

Fix failing tests.
Updated message for tweak from a specified base class.

Fri, Jan 22, 4:25 PM · Restricted Project
njames93 committed rGd18c3c7b18e9: [CodeComplete] Add ranged for loops code pattern. (authored by njames93).
[CodeComplete] Add ranged for loops code pattern.
Fri, Jan 22, 3:41 PM
njames93 closed D95131: [CodeComplete] Add ranged for loops code pattern..
Fri, Jan 22, 3:40 PM · Restricted Project
njames93 requested review of D95270: [clangd][NFC] Simplify handing on methods with no params.
Fri, Jan 22, 3:39 PM · Restricted Project
njames93 added inline comments to D94942: [clangd] Add tweak for implementing abstract class.
Fri, Jan 22, 10:28 AM · Restricted Project
njames93 updated the summary of D94942: [clangd] Add tweak for implementing abstract class.
Fri, Jan 22, 9:24 AM · Restricted Project
njames93 updated the diff for D94942: [clangd] Add tweak for implementing abstract class.

If cursor is over one of the base specifiers, offer to implement only the pure methods from that base class.
Depends on D95231

Fri, Jan 22, 9:23 AM · Restricted Project
njames93 updated the diff for D95231: [clangd] Selection handles CXXBaseSpecifier.

Update FindTargets to fix symbol renaming on a CXXBaseSpecifier.
Add test to ensure rename wont trigger on a CXXBaseSpecifier.

Fri, Jan 22, 7:34 AM · Restricted Project
njames93 requested review of D95231: [clangd] Selection handles CXXBaseSpecifier.
Fri, Jan 22, 6:44 AM · Restricted Project
njames93 added a comment to D94942: [clangd] Add tweak for implementing abstract class.

Thanks this looks great, and something i've been longing for some time! But I got a couple of questions about the how the interaction is designed (sorry if I missed some high level discussions elsewhere, feel free to just point me in that direction).

There was no high level discussions on my end. I'm just a guy on my own, who makes what I like, I don't even work in software. Though if there is a public place where people seem to discuss these I'd like to be pointed in the direction(discord seems to be mainly used for user support and clangd-dev has only had one relevant post in the last 6 months). Right now seems that GitHub issues and here are my best bets.

Fri, Jan 22, 6:36 AM · Restricted Project

Thu, Jan 21

njames93 added inline comments to D94942: [clangd] Add tweak for implementing abstract class.
Thu, Jan 21, 2:17 PM · Restricted Project
njames93 updated the diff for D94942: [clangd] Add tweak for implementing abstract class.

Split up the code a little more. Fix a few malformed comments.

Thu, Jan 21, 2:15 PM · Restricted Project
njames93 requested review of D95131: [CodeComplete] Add ranged for loops code pattern..
Thu, Jan 21, 6:07 AM · Restricted Project

Wed, Jan 20

njames93 updated the diff for D94942: [clangd] Add tweak for implementing abstract class.
  • Replace getFinalOverrides for a manual implementation, that method wasn't quite suited to what was needed w.r.t tracking access.
  • Add support for template classes with no dependant bases.
  • Add tests for template classes.
Wed, Jan 20, 1:02 PM · Restricted Project
njames93 requested review of D95046: [clangd] Add option to use dirty file contents when building preambles..
Wed, Jan 20, 6:54 AM · Restricted Project
njames93 retitled D94554: [clangd] Add a Filesystem that overlays Dirty files. from [clangd][WIP] Add a Filesystem that overlays Dirty files. to [clangd] Add a Filesystem that overlays Dirty files..
Wed, Jan 20, 6:47 AM · Restricted Project
njames93 retitled D93978: [clangd] Use dirty filesystem when performing cross file tweaks from [clangd] DefineOutline doesn't require implementation file being saved to [clangd] Use dirty filesystem when performing cross file tweaks.
Wed, Jan 20, 6:35 AM · Restricted Project
njames93 updated the diff for D93978: [clangd] Use dirty filesystem when performing cross file tweaks.

Fixed last series being a little corrupted.

Wed, Jan 20, 6:33 AM · Restricted Project
njames93 updated the diff for D93978: [clangd] Use dirty filesystem when performing cross file tweaks.

Change implementation to use DirtyFS instead.

Wed, Jan 20, 6:27 AM · Restricted Project
njames93 requested review of D95043: [clangd] Use Dirty Filesystem for cross file rename..
Wed, Jan 20, 6:22 AM · Restricted Project
njames93 updated the diff for D94554: [clangd] Add a Filesystem that overlays Dirty files..

Start Splitting this patch up

Wed, Jan 20, 6:09 AM · Restricted Project

Tue, Jan 19

njames93 planned changes to D93978: [clangd] Use dirty filesystem when performing cross file tweaks.

I've decided I much prefer the implementation in D94554, So once thats cleaned up and split up, I'll merge changes in here with there.

Tue, Jan 19, 3:48 PM · Restricted Project
njames93 updated the diff for D94942: [clangd] Add tweak for implementing abstract class.

Update printing policy.

Tue, Jan 19, 4:27 AM · Restricted Project

Mon, Jan 18

njames93 updated the diff for D94942: [clangd] Add tweak for implementing abstract class.

Forgot to add the untracked files

Mon, Jan 18, 8:42 PM · Restricted Project
njames93 requested review of D94942: [clangd] Add tweak for implementing abstract class.
Mon, Jan 18, 8:41 PM · Restricted Project
njames93 added reviewers for D94554: [clangd] Add a Filesystem that overlays Dirty files.: sammccall, kadircet.
Mon, Jan 18, 5:50 AM · Restricted Project

Sat, Jan 16

njames93 requested changes to D94131: [clang-tidy] Use new mapAnyOf matcher.

Can you either update the description of this patch to include the binaryOperation changes, or remove those changes from here.

Sat, Jan 16, 1:27 PM · Restricted Project

Fri, Jan 15

njames93 added a comment to D94785: [clangd] Index local classes, virtual and overriding methods..

Thank you for taking the time to fix this and the performance measurements to boot.
My only question is does this try and index lambdas, under the hood they are declared as a CXXRecordDecl, defined in function scope?

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

Thu, Jan 14

njames93 added a reviewer for D94621: [clang-tidy] add concurrency-async-fs: aaron.ballman.

Is this just flagging all these functions, if so I don't think there's much value in here.

Thu, Jan 14, 4:40 AM · Restricted Project, Restricted Project
njames93 accepted D94601: [clang-tidy] Use DenseSet<SourceLocation> in UpgradeDurationConversionsCheck, NFCI.

Seems reasonable, LGTM.

Thu, Jan 14, 4:37 AM · Restricted Project, Restricted Project
njames93 updated the diff for D94554: [clangd] Add a Filesystem that overlays Dirty files..

Refactor RefCntString out of DraftStore class
Remove MTime from Draft, other users of DraftStore don't need to see it.

Thu, Jan 14, 2:28 AM · Restricted Project

Wed, Jan 13

njames93 added a comment to D93452: [clangd] Trim memory periodically.

A little follow up, but I've noticed if I have a lot of tabs open in my editor (usually happens as I forget to close them).
When clangd starts (or restarts) , in the first minute, memory usage will shoot right up as it starts to do its thing.
I've regularly seen it go over 15GB before the first trim call is made and it drops back down and settling at ~2GB. For reference clangd typically reports its usage is ~1.5GB when I'm working on LLVM.
This does result in some noticeable hanging of my system, which while not workstation level, its got more horsepower than a chromebook, no offence to anyone who works at Google :).

Wed, Jan 13, 6:21 PM · Restricted Project, Restricted Project
njames93 committed rGaf1bb4bc823f: Fix build errors after ceb9379a9 (authored by njames93).
Fix build errors after ceb9379a9
Wed, Jan 13, 4:20 AM
njames93 committed rGceb9379a90f5: [ADT] Fix join_impl using the wrong size when calculating total length (authored by njames93).
[ADT] Fix join_impl using the wrong size when calculating total length
Wed, Jan 13, 3:37 AM
njames93 closed D83305: [ADT] Fix join_impl using the wrong size when calculating total length.
Wed, Jan 13, 3:37 AM · Restricted Project
njames93 updated the diff for D94554: [clangd] Add a Filesystem that overlays Dirty files..

Remove some unrelated changes

Wed, Jan 13, 3:35 AM · Restricted Project

Tue, Jan 12

njames93 updated the diff for D83305: [ADT] Fix join_impl using the wrong size when calculating total length.

Add assert to enusre correct behaviour.

Tue, Jan 12, 3:57 PM · Restricted Project
njames93 added a comment to D83305: [ADT] Fix join_impl using the wrong size when calculating total length.

The spec seems to be a bit more precise than that (using cppreference, which isn't authoritative, but more readable): "If new_cap is greater than the current capacity(), new storage is allocated, and capacity() is made equal or greater than new_cap."

So a unit test that joins some large strings (if it wanted to be fancy, it could even generate a string intentionally larger than std::string's default capacity, whatever it happens to be) and if the resulting string has a larger capacity than std::string's default capacity, verify that the capacity is exactly equal to the string length.

That still sounds like it'll cause some issues. The main thing we want to test (which is the purpose of this patch) is that the string didn't grow while we are appending all the pieces. It's impossible to tell externally if that happened as reserve may choose to over allocate.
Putting an assert in the function that the capacity before hasn't changed should be more than enough, WDYT?

Tue, Jan 12, 3:56 PM · Restricted Project
njames93 committed rGa7130d85e4b9: [ADT][NFC] Use empty base optimisation in BumpPtrAllocatorImpl (authored by njames93).
[ADT][NFC] Use empty base optimisation in BumpPtrAllocatorImpl
Tue, Jan 12, 2:44 PM
njames93 closed D94439: [ADT][NFC] Use empty base optimisation in BumpPtrAllocatorImpl.
Tue, Jan 12, 2:44 PM · Restricted Project
njames93 planned changes to D94554: [clangd] Add a Filesystem that overlays Dirty files..
Tue, Jan 12, 2:37 PM · Restricted Project
njames93 requested review of D94554: [clangd] Add a Filesystem that overlays Dirty files..
Tue, Jan 12, 2:20 PM · Restricted Project

Mon, Jan 11

njames93 updated the summary of D94439: [ADT][NFC] Use empty base optimisation in BumpPtrAllocatorImpl.
Mon, Jan 11, 1:05 PM · Restricted Project
njames93 added a comment to D94439: [ADT][NFC] Use empty base optimisation in BumpPtrAllocatorImpl.

I think for full generality the standard library does some things to check if a type can be derived from before using it in this way - but we probably don't need all that machinery for LLVM-internal allocator stuff like this?

We just have to make sure people don't use an allocator that's defined with the final keyword. I doubt we are going to run into many cases of that.
I have done the template magic to get it make it work if someone wants to use an allocator marked final, should I include it?

Probably wouldn't bother - might be worth leaving it here or in the commit message, or in a comment in the code as a "here's how to do it if it turns out someone needs it".

To make the class work is code than I'd be happy to tack onto the commit message. I'll just make a note in the commit message about it.
Basically just create a class that has an instance of the final Allocator type, then create calls that forward to that instance.

Mon, Jan 11, 1:01 PM · Restricted Project
njames93 added a comment to D94439: [ADT][NFC] Use empty base optimisation in BumpPtrAllocatorImpl.

I think for full generality the standard library does some things to check if a type can be derived from before using it in this way - but we probably don't need all that machinery for LLVM-internal allocator stuff like this?

We just have to make sure people don't use an allocator that's defined with the final keyword. I doubt we are going to run into many cases of that.
I have done the template magic to get it make it work if someone wants to use an allocator marked final, should I include it?

Mon, Jan 11, 12:50 PM · Restricted Project
njames93 committed rGd3ff24cbf872: [ADT] Add makeIntrusiveRefCnt helper function (authored by njames93).
[ADT] Add makeIntrusiveRefCnt helper function
Mon, Jan 11, 12:13 PM
njames93 closed D94440: [ADT] Add makeIntrusiveRefCnt helper function.
Mon, Jan 11, 12:13 PM · Restricted Project
njames93 updated the diff for D94440: [ADT] Add makeIntrusiveRefCnt helper function.

Address comments.

Mon, Jan 11, 12:12 PM · Restricted Project
njames93 added a comment to D94390: [clangd] Extend find-refs to include overrides..

Should we be providing virtual method overrides when querying definitions, surely that's what query implementations is for?

Mon, Jan 11, 12:03 PM · Restricted Project
njames93 added inline comments to D94440: [ADT] Add makeIntrusiveRefCnt helper function.
Mon, Jan 11, 11:49 AM · Restricted Project
njames93 requested review of D94440: [ADT] Add makeIntrusiveRefCnt helper function.
Mon, Jan 11, 11:48 AM · Restricted Project
njames93 requested review of D94439: [ADT][NFC] Use empty base optimisation in BumpPtrAllocatorImpl.
Mon, Jan 11, 11:27 AM · Restricted Project
njames93 committed rG31732e6f52c8: [clangd] Remove ScratchFS from tests (authored by njames93).
[clangd] Remove ScratchFS from tests
Mon, Jan 11, 8:14 AM
njames93 closed D94359: [clangd] Remove ScratchFS from tests.
Mon, Jan 11, 8:14 AM · Restricted Project
njames93 updated the summary of D94359: [clangd] Remove ScratchFS from tests.
Mon, Jan 11, 2:08 AM · Restricted Project
njames93 updated the diff for D94359: [clangd] Remove ScratchFS from tests.

Remove extra semi-colon.

Mon, Jan 11, 2:08 AM · Restricted Project

Sun, Jan 10

njames93 added inline comments to D94382: [clangd] Avoid recursion in TargetFinder::add().
Sun, Jan 10, 6:06 PM · Restricted Project
njames93 retitled D94382: [clangd] Avoid recursion in TargetFinder::add() from [clangd] Avoid recusion in TargetFinder::add() to [clangd] Avoid recursion in TargetFinder::add().
Sun, Jan 10, 5:56 PM · Restricted Project
njames93 added a comment to D93978: [clangd] Use dirty filesystem when performing cross file tweaks.

So I did some tweaking and I have a snapshot filesystem that will hold shared references to the contents, thereby avoiding the need to copy the file contents.
It works for rename and tweaks with no visible issues.
Reasoning behind this a long term goal of letting a user choose between using on disk or dirty buffers when building preambles.
Obviously there, if using dirty buffers, we wouldn't want to be copying every open file (even unused files) during preamble building/checking.

Sun, Jan 10, 4:56 PM · Restricted Project

Sat, Jan 9

njames93 updated the diff for D94359: [clangd] Remove ScratchFS from tests.

Fix windows tests failing(hopefully).

Sat, Jan 9, 5:28 AM · Restricted Project
njames93 requested review of D94359: [clangd] Remove ScratchFS from tests.
Sat, Jan 9, 4:45 AM · Restricted Project

Fri, Jan 8

njames93 committed rG467cbd298184: [clangd][NFC] Remove unnecessary copy in CodeComplete (authored by njames93).
[clangd][NFC] Remove unnecessary copy in CodeComplete
Fri, Jan 8, 6:32 PM
njames93 added a comment to D94293: [clangd] automatically index STL.

How does this approach work with differing language standards. For example with an old implementation designed for c++14, string_view header won't exist. If the implementation is designed to work with c++17, including that header in c++14 mode will probably be ifdef... Error.

Fri, Jan 8, 4:55 AM · Restricted Project

Thu, Jan 7

njames93 updated the diff for D93978: [clangd] Use dirty filesystem when performing cross file tweaks.

Address most comments.

Thu, Jan 7, 2:01 PM · Restricted Project
njames93 added inline comments to D93978: [clangd] Use dirty filesystem when performing cross file tweaks.
Thu, Jan 7, 1:59 PM · Restricted Project
njames93 updated the summary of D93978: [clangd] Use dirty filesystem when performing cross file tweaks.
Thu, Jan 7, 1:55 AM · Restricted Project
njames93 updated the diff for D93978: [clangd] Use dirty filesystem when performing cross file tweaks.

Use VFS instead of DraftStore

Thu, Jan 7, 1:55 AM · Restricted Project
njames93 abandoned D93977: [clangd] Pass DraftStore to Tweak Apply and Prepare..

Abandoning in favour of VFS approach

Thu, Jan 7, 1:26 AM · Restricted Project

Wed, Jan 6

njames93 added a comment to D93978: [clangd] Use dirty filesystem when performing cross file tweaks.

Ah, thanks for working on this!

A few thoughts:

  • when we're pseudoparsing the file we're going to modify as we do here, using the new content is strictly better, no downside :-)
  • the old method here of accessing the content through the FS attached to the AST is a hack
  • however I think DraftStore is the wrong abstraction here and VFS is the right one.
    • each tweak shouldn't have to deal with falling back from DraftStore to VFS (and rename also has an implementation of this!)
    • C++ embedders don't have a DraftStore lying around
    • bug: this code won't find a named but unsaved implementation file (since getCorrespondingHeaderOrSource uses VFS)
  • (also Tweak::Selection isn't a pure abstraction, we can jam the FS in there instead of passing it around as a separate param everywhere)

So I think the right way to expose this to tweaks is to add a vfs::FileSystem * member to Tweak::Selection, that allows accessing the current FS (as opposed to the one used for building).
The simplest implementation uses OverlayVFS + InMemoryFS + TUScheduler::getAllFileContents(). This isn't as expensive as it sounds as we need only set the FS pointer for apply().
(We can also implement a FS that copies more lazily, though I suspect it's not worth it)

Wed, Jan 6, 6:52 PM · Restricted Project
njames93 committed rG3505d8dc0742: [clangd][NFC] Use PathRef for getCorrespondingHeaderOrSource (authored by njames93).
[clangd][NFC] Use PathRef for getCorrespondingHeaderOrSource
Wed, Jan 6, 6:42 PM
njames93 committed rG0bfe10014563: [NFC] Test case refactor (authored by njames93).
[NFC] Test case refactor
Wed, Jan 6, 12:00 PM
njames93 added a comment to D94131: [clang-tidy] Use new mapAnyOf matcher.

Theres a few compile errors here in the pre merge. Is this patch based against trunk or some local branch?

Wed, Jan 6, 3:54 AM · Restricted Project

Sun, Jan 3

njames93 added a comment to D93979: [clang-tidy] Fix windows tests.

http://45.33.8.238/win/30678/summary.html It looks like it works, as nothing needed to be built it was a v fast build

Sun, Jan 3, 4:44 PM · Restricted Project
njames93 committed rG59810c51e761: [clang-tidy] Fix windows tests (authored by njames93).
[clang-tidy] Fix windows tests
Sun, Jan 3, 4:40 PM
njames93 closed D93979: [clang-tidy] Fix windows tests.
Sun, Jan 3, 4:39 PM · Restricted Project

Sat, Jan 2

njames93 added a comment to D93979: [clang-tidy] Fix windows tests.

I don't have a (reliable) windows machine to test so can you take a look please

Sat, Jan 2, 3:54 PM · Restricted Project
njames93 requested review of D93979: [clang-tidy] Fix windows tests.
Sat, Jan 2, 3:53 PM · Restricted Project
njames93 committed rG7af6a134508c: [NFC] Switch up some dyn_cast calls (authored by njames93).
[NFC] Switch up some dyn_cast calls
Sat, Jan 2, 11:57 AM
njames93 requested review of D93978: [clangd] Use dirty filesystem when performing cross file tweaks.
Sat, Jan 2, 10:14 AM · Restricted Project
njames93 requested review of D93977: [clangd] Pass DraftStore to Tweak Apply and Prepare..
Sat, Jan 2, 10:04 AM · Restricted Project

Wed, Dec 30

njames93 added inline comments to D93844: [clang-format] Add possibility to be based on parent directory.
Wed, Dec 30, 6:56 AM · Restricted Project, Restricted Project
njames93 added a comment to D93940: [clang-tidy] Add a check for blocking types and functions..

Few stylistic nits, Also Theres lots of cases where single stmt if statements have braces, typically we elide those braces.
Is it worth flagging methods with Thread safety analysis attributes.
These are only used in clang, but if a project annotates their methods with these, it would be nice to autodetect these attributes, though I can see this being a big job and likely better for a follow up.

Wed, Dec 30, 5:14 AM · Restricted Project, Restricted Project

Tue, Dec 29

njames93 added a comment to D83305: [ADT] Fix join_impl using the wrong size when calculating total length.

Any chance of testing this? Could test the 'capacity' after join to check its the right size?

How do you suppose I do that, maybe expensive checks to get the capacity after reserve and after all items have been added and ensure they are the same, otherwise the string grew during appending.

Oh, nah, nothing that'd require expensive checks - you could add an assert that size == Len at the end of the function (would have to have some way of avoiding the case for short results that are below the starting capacity, though... perhaps checking if capacity grew over the reserve call, and only asserting in that case)). But I was thinking more along the lines of a unit test that tests join and checks the capacity isn't bigger than the size for a few hardcoded samples.

Oh that assert is fine, the unit test won't be a good test. As we don't control std string, it's capacity for calls to reserve will be implementation defined

Tue, Dec 29, 3:30 PM · Restricted Project
njames93 added a comment to D83305: [ADT] Fix join_impl using the wrong size when calculating total length.

Any chance of testing this? Could test the 'capacity' after join to check its the right size?

Tue, Dec 29, 3:14 PM · Restricted Project

Mon, Dec 28

njames93 added a reviewer for D83305: [ADT] Fix join_impl using the wrong size when calculating total length: dblaikie.

Ping.

Mon, Dec 28, 1:11 PM · Restricted Project
njames93 accepted D93653: [clangd] Avoid reallocating buffers for each message read:.

I'm easy with this.

Mon, Dec 28, 12:07 PM · Restricted Project
njames93 updated the diff for D93861: [clang-tidy][NFC] Split up some headers.

Fix clang-tidy lint and comments.

Mon, Dec 28, 11:43 AM · Restricted Project
njames93 requested review of D93861: [clang-tidy][NFC] Split up some headers.
Mon, Dec 28, 8:18 AM · Restricted Project
njames93 committed rGc3b9d85bd4b7: [clang-tidy][NFC] Remove unnecessary headers (authored by njames93).
[clang-tidy][NFC] Remove unnecessary headers
Mon, Dec 28, 7:02 AM

Sun, Dec 27

njames93 added a comment to D93844: [clang-format] Add possibility to be based on parent directory.

I'm a little confused why this is needed as clang-format always read up the directory tree until it see a .clang-format file, perhaps I don't quite understand the use case. Can't you just have a different .clang-format in the subdirectory?

Sun, Dec 27, 3:12 PM · Restricted Project, Restricted Project

Sat, Dec 26

njames93 committed rG62beac7ed7c6: [NFC] Refactor some SourceMgr code (authored by njames93).
[NFC] Refactor some SourceMgr code
Sat, Dec 26, 9:54 AM
njames93 updated njames93.
Sat, Dec 26, 9:17 AM

Dec 24 2020

njames93 requested review of D93800: [clangd][WIP] Add caching behaviour for clang-format config.
Dec 24 2020, 3:26 AM · Restricted Project
njames93 added a comment to D93761: [libObject/Decompressor] - Use `resize_for_overwrite` in Decompressor::resizeAndDecompress()..

IMO this may be misleading. This appears to be faster because previously memset zero was called on the buffer which triggered page faults and allocates the pages immediately.
With the new change the time is amortized with the decompression itself. I think we should measure the total time with decompression.

Be interesting to see the difference, would be nice to query performance of decompression alone as well. See how much slowdown there is if pages haven't been allocated eagerly.

Dec 24 2020, 1:47 AM · Restricted Project

Dec 23 2020

njames93 added a comment to D93763: [clangd] Add a flag to disable the documentLinks LSP request.

Can you apply the clang-format patch suggested by the pre-merge bot. Alternatively, if you have a recent version of clang-format and the appropriate git-hooks you should be able to run git clang-format for the same effect.

Dec 23 2020, 2:57 PM · Restricted Project
njames93 committed rGc7e825b910a9: [format][NFC] Use unsigned char as the base of all enums in FormatStyle (authored by njames93).
[format][NFC] Use unsigned char as the base of all enums in FormatStyle
Dec 23 2020, 12:28 PM
njames93 closed D93758: [format][NFC] Use unsigned char as the base of all enums in FormatStyle.
Dec 23 2020, 12:28 PM · Restricted Project
njames93 added a comment to D93758: [format][NFC] Use unsigned char as the base of all enums in FormatStyle.

I don't have any major issues with this other than it makes Format.h a bit more ugly. (there are more ugly and harder to understand pieces of code than this change!)

We'll only normally really have 1 of these in play at any one time, but it is passed around quite a bit on the stack, should we care about the extra space it's taking? (I'm ok if the answer is "yes we should")

Dec 23 2020, 8:29 AM · Restricted Project
njames93 updated the diff for D93758: [format][NFC] Use unsigned char as the base of all enums in FormatStyle.
  • Update dump_format_style script

Only tweaked regex to support enum <name> : <words> ... {
Decided there wasn't much to gain from supporting nested types etc

Dec 23 2020, 7:53 AM · Restricted Project
njames93 added a comment to D93758: [format][NFC] Use unsigned char as the base of all enums in FormatStyle.

We should probably check the docs/tools/dump_format_style.py still works

Good catch, I'll update the script to optionally take an underlying type argument. Should that be part of this change, or a parent?

Dec 23 2020, 6:41 AM · Restricted Project