User Details
- User Since
- Feb 5 2013, 8:31 AM (528 w, 2 d)
Mon, Mar 13
Fri, Mar 10
Tue, Mar 7
Wed, Mar 1
I guess it was originally done this way because we have a number of DependencyScanning{Tool,Worker} instances
Feb 21 2023
LGTM. The vfsroot-include.c is a -cc1 invocation, so it could probably be switched to use -verify instead of FileCheck'ing the output, unless there's something I'm missing. But that doesn't need to be for this patch.
Feb 14 2023
Feb 10 2023
Since perf impact is minimal, LGTM.
Feb 9 2023
Add a test
Feb 8 2023
I think we should verify the performance impact to running tests is minimal before landing this, like they did with the _GLIBCXX_ASSERTIONS change. Also CCing @ldionne in case he has opinions on us using libc++ assertions. Otherwise LGTM.
- Improved doc comment for RoundTrip (mostly followed the suggested text; also converted from to /).
- Renamed variables
- Moved default value for RoundTripArgs
+1 for determinism. Okay I spent some time trying to understand how this code is used and the non-virtual paths make some sense now. I am a bit skeptical about this on-disk-hash-table by filepath but that's separate from this patch.
Feb 7 2023
This should allow the path serialization of input files to use the paths used when looking up a file entry, instead of the last reference.
Dupe of https://reviews.llvm.org/D143428 ?
Feb 6 2023
Feb 3 2023
Jan 31 2023
Jan 27 2023
Iterating over SeenFileEntries and skipping VirtualFileEntries looks good to me.
Jan 24 2023
Jan 23 2023
Removing ImplicitModulePCMPath LGTM; not sure about the other change.
Jan 20 2023
I'm not that familiar with development practices for libunwind, but is possible to write a test for this?
Jan 19 2023
Jan 13 2023
Please add a comment to each place you're doing this since it's non-obvious why the filename would be different to people not familiar with the mapping code. Otherwise LGTM
Jan 6 2023
Sorry for missing these. I think I addressed all of them now.
Jan 5 2023
Re-added a few of my minor comments that I think got lost on a specific version of the diff. Otherwise LGTM.
Jan 3 2023
Dec 12 2022
libclang changes LGTM, thanks! I'll let the other reviewers who have domain expertise speak for the rest of the patch.
Dec 9 2022
Thanks, the libclang change LGTM other than my comment about the function names.
Dec 8 2022
Dec 7 2022
All my comments are addressed, thanks!
Dec 2 2022
Thanks for the explanation, make sense. LGTM with the documentation tweak!
If the original HeaderSearchOptions didn't have any VFS overlay files, adopt the PCM ones.
Dec 1 2022
Please add a comment about why this equality check is necessary.
Nov 30 2022
Nov 28 2022
Nov 18 2022
LGTM once the test failure is fixed
Nov 17 2022
Nov 16 2022
Nov 15 2022
- Removed outdated FIXME
- Ran clang-format
- Attempt to fix Windows test failure: the StringSet of paths was failing to match, so switch to DenseSet<FileEntry *>. Note: the path that ends up in the command-line is from a FileEntryRef, but the equality of FileEntry * is how we check whether to include it at all. For now we just re-lookup the paths which should always hit the path cache in FIleManager. A future improvement could be to track FileEntryRef for the module map paths directly instead of strings.
Nov 14 2022
Nov 11 2022
Nov 2 2022
Thanks for the timely review!