njames93 added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp:81
+    if (InsertedHeaders[FileID].contains(Header))
+      return llvm::None;
+  } else if (!InsertedHeaders[FileID].insert(Header).second)
----------------
steveire wrote:
> The comment and the code seem to me to be opposites. The code says "if 
> `SingleFixMode` is true and we have already inserted the header,  don't 
> insert it again." The comment says that "`SingleFixMode` does not prevent 
> inserting it again" -- The comment seems to be the opposite of the code?
That't not what the code is saying, the magic is clear in the else branch. If 
SingleFixMode is enabled we are only querying the set, If its disabled then we 
are updating the set. 
The real shortfall is the map is called InsertedHeaders which is not a good 
reflection of what it actually contains. It actually contains all the headers 
included in each file. Which in `Multi` fix mode gets updated with each include 
insertion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97121

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to