- User Since
- Aug 26 2016, 6:53 AM (156 w, 9 h)
I think we concluded we wanted to merge this into the release, which is apparently still possible. @hans
kick phabricator to try to get it to mail
Wed, Aug 21
revert extraneous include
Right, the sequence is:
- PrintTo() is found by overload resolution, if there's no specialized version, it falls back to the generic template
- the generic template checks whether it's a container (by looking for nested iterator and const_iterator types)
- for non-containers, it attempts to use operator<< if it exists, falling back to "n-byte object XX-XX-XX-XX" otherwise
- for containers, it prints each element in the same manner
So overloading operator<< doesn't bypass the container detection, but overloading PrintTo() does. And this is what gtest does for std::string, in gtest-printers.h. I think we should do the same as a local modification to internal/custom/gtest-printers.h.
Tue, Aug 20
Main themes are:
- there are a few places we can reduce scope for the first patch: e.g. template support
- internally, it'd be clearer to pass data around in structs and avoid complex classes unless the abstraction is important
- high-level documentation would aid in understanding: for each concept/function: what is the goal, how does it fit into higher-level goals, are there any non-obvious reasons it's done this way
- naming questions (this can solve some of the same problems as docs, but much more cheaply)
Thu, Aug 15
This is glorious, thank you!
Wed, Aug 14
+ilya because I know he's wrestled with these before, and because I'm OOO sick
Mon, Aug 12
Added a bit of a braindump to https://github.com/clangd/clangd/issues/95
Thanks! I actually like the ASSERT_NO_ERROR macros, std::error_code() meaning success is pretty obscure to write everywhere.
Sorry, I had marked this as "accepted", but not hit send. I agree with Ilya's comments, but please go ahead and land this when ready.
Agreed, this seems fine as is.
Fri, Aug 9
@hans If you don't mind merging this, it's a nice usability improvement.
Do we have real examples of code that gets formatted incorrectly when parsed as c++20 but lives in a codebase that uses coroutines TS?
This seems very niche (and probably short-lived) - do we really need this option?
This looks reasonable to me, there are a couple of variations you might think about:
- also treat QualifiedNameLookup as an option, and override in places with an RAII pattern like ScopedOverride Unqual(QualifiedNameLookup, false) (why don't we have a class like that?). This would further reduce args.
- put the options in a LookupOpts struct and pass it by const reference - this is a less invasive change
ugh, I accidentally reused a branch and squashed two patches together. Let me try again...
Thanks! Want me to land this for you?
Sorry for taking so long here, I've been swamped.
Yeah, this is much better than only having -background-index=no.
Renamed in rL368455
@hans is there still time to merge this?
Thanks for the heads up!
I think the breakage was rather from D62271 (when this was written, there was no plan to switch the default)
Thu, Aug 8
Wed, Aug 7
Only hesitation is that AllTUsExecution is one executor of (potentially) many, so this is a weird place for a common flag.
Tue, Aug 6
Sorry, should have accepted this in the last round. Great stuff, thank you!
I'd like to know if you tried other approaches and why they failed.
Mon, Aug 5
Oops, didn't mean to mark as accepted.
It seems conceptually a little strange to have the working directory be part of a serialized "FS", as it's fundamentally a property of a process and only transiently a property of the VFS.
I don't know this is fatal (not sure what else the RedirectingFileSystem might be used for.
Fri, Aug 2
This appears to have missed lib/Index/SimpleFormatContext.h.
AFAICT it's unused and untested, but it used to parse and now it doesn't. It should probably either be updated or removed from the tree.
Thanks, this looks a lot better. There are a bunch of details that can be simplified and names that could be clearer, but I think this is the right shape.
I guess you're looking for design review at this point.
I'm lacking a bit of context here.
Why do we prefer this to EXPECT_FALSE(err) /EXPECT_EQ(err, std::errc_address_in_use) with an appropriate value printer for std::error_code?