[PATCH] D123197: Remove a few effectively-unused FileEntry APIs. NFC

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 6 11:22:47 PDT 2022


dexonsmith added inline comments.


================
Comment at: clang/lib/Basic/FileManager.cpp:462
   UFE->Dir     = &DirInfo->getDirEntry();
-  UFE->UID     = NextFileUID++;
-  UFE->IsValid = true;
+  UFE->UID = NextFileUID++;
   UFE->File.reset();
----------------
sammccall wrote:
> kadircet wrote:
> > sammccall wrote:
> > > kadircet wrote:
> > > > nit: revert formatting
> > > `arc` insists on this change here, because it's in the blast radius of the next line and clang-format isn't configured to allow this formatting.
> > > 
> > > Unless you feel strongly I'd rather not fight the linter when it's not really wrong.
> > > (Happy to reformat the other lines to match though)
> > > Unless you feel strongly I'd rather not fight the linter when it's not really wrong.
> > > (Happy to reformat the other lines to match though)
> > 
> > I was suggesting this for preserving history purposes, in theory all of this is old code with intricate semantics so it would be nice to not add more commits to the history chain here. I think it should be possible to just `arc diff --nolint` (or push without updating the diff in phabricator, which you already probably know, but just in case).
> Sure, fair enough
> `arc` insists on this change here, because it's in the blast radius of the next line and clang-format isn't configured to allow this formatting.
> 
> Unless you feel strongly I'd rather not fight the linter when it's not really wrong.
> (Happy to reformat the other lines to match though)

I know this was already reverted / dealt with (SGTM; I agree with @kadircet, although probably I feel less strongly), but I'm wondering if there's a bug in `arc` or `clang-format` to report or fix. Seems to me like this *shouldn't* be considered part of the blast radius. Why should deleting a line trigger reformatting the lines before/after, when those lines are not part of the same construct/statement?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123197/new/

https://reviews.llvm.org/D123197



More information about the cfe-commits mailing list