kadircet added inline comments.

================
Comment at: clang/include/clang/Tooling/Inclusions/Header.h:1
+//===--- Header.h ------------------------------------------------*- 
C++-*-===//
+//
----------------
rather than calling this `Header` can we call this `HeaderAnalysis`?


================
Comment at: clang/include/clang/Tooling/Inclusions/Header.h:21
+///
+/// A header is considered self-contained if either it has a proper header 
guard
+/// or it doesn't has dont-include-me-similar pattern.
----------------
let's mention being `#import`d as well


================
Comment at: clang/lib/Tooling/Inclusions/Header.cpp:64
+  // particular preprocessor state, usually set up by another header.
+  return !isDontIncludeMeHeader(SM.getBufferData(SM.translateFile(FE)));
+}
----------------
`translateFile` actually does a linear scan over all the slocentries, so i 
think it's better for this API to be based on FileID. (later on we can easily 
get fileentry from fileid in constant time)


================
Comment at: clang/unittests/Tooling/HeaderTest.cpp:21
+  Inputs.Code = R"cpp(
+  #include "headerguard.h"
+  #include "pragmaonce.h"
----------------
can you also add a test case for `#import` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137697

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

Reply via email to