- User Since
- Aug 26 2016, 6:53 AM (178 w, 10 h)
Good point about the interaction with bin-packing.
Do the style guides that want this actually endorse bin-packing containers? Insertion + bin-packing + comma-as-hint basically can't coexist - we could turn off the hint or make this option mutually exclusive with bin-packing.
For rolling this out, I think the best path is:
- check in the option, but don't turn it on with any styles yet
- test it by taking a large codebase, formatting it (normal options), format it (with comma insertion), look at the diffs (internally google3diff can do this)
- turn it on for -style=google
Can you file a new bug or modify the upstream one to audit existing uses of getTemplateInstantiationPattern() (and maybe related functions)?
Document new interface and threading model of existing functions.
oops, needs some more tests.
Nit: maybe mention "top-level" in the patch?
Wed, Jan 22
revert accidental change
Revert changes to VSCode client. This experimental version of the VSCode libs
is fairly new and some corp mirrors we care about are behind ;-)
Let's make the minimal change here and land this.
Tue, Jan 21
I've tried this out locally and it's fun! As suspected on the bug though, IMO it's far from accurate enough. Examples from clangd/Compiler.cpp:
- it triggers on almost every word, even words that plainly don't refer to any decl like format [[lazily]], in case vlog is off. This means that e.g. (in VSCode) the underline on ctrl-hover gives no/misleading signal. It also means that missing your target now jumps you somewhere random instead of doing nothing.
- when it works properly, the correct result usually mixed with incorrect results (e.g. createInvocationFromCommandLine sets [[DisableFree]]).
- it doesn't work for some symbols - ones that are not indexable (e.g. RemappedFileBuffers will handle the lifetime of the [[Buffer]] pointer, gives a variety of wrong results)
Thanks for fixing this. I think it could maybe use a further tweak, LMK if I'm missing something as my intuition around macros is pretty bad.
Fri, Jan 17
Thu, Jan 16
Please revert this, it was already fixed by d54d71b67e60
kadir has a pending (or landed?) fix for this.
In this patch, relative paths are relative to the working directory (or at least the current directory of the VFS).
Thanks for fixing this.
Tue, Jan 14
Mon, Jan 13
This appears to have broken bounds checking in some cases.
There don't seem to be any in-tree tests, but the exegesis DisassemblerTest.TooShortABuffer hits an LLVM assertion:
Following crashes for me with clang++ -g2:
This seems to have introduced a crash compiling libcxx. I'm currently reducing the crashing code.
Fri, Jan 10
I think i'm also comfortable with marking the linked bug as wontfix.
Basing this on the hover for the type doesn't seem right. e.g. int should be the Type rather than the Name.
Yeah we discussed that case offline and I should have mentioned.
Resolving this in places other than the top-level would be nice and probably worthy of a comment at least. But this special case seems common enough to be worth having.
The only place fully resolving could reasonably be done I guess is TypePrinter, because we can't actually transform the types into the correct form IIUC.
It's particularly unclear to me why typeprinter descends into auto but prints decltype, but Kadir says that seems to be intentional.
By which I mean, adding the same delegation to the RAV there doesn't work, and I'm not confident that jiggling it around until the test passes is correct here, so I want to understand the traversal behavior a bit better first (RAV has several special cases around OVE/POE)
Thu, Jan 9
still LG, go ahead and we can iterate
Wed, Jan 8
I wonder what accident of history led to the different number of dashes between the flags, weird.
Great stuff, let's finally ship it!