kadircet added inline comments.
================ 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. ---------------- kadircet wrote: > let's mention being `#import`d as well again comment seems to be the same. ================ 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))); +} ---------------- sammccall wrote: > kadircet wrote: > > `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) > You can get content through the fileentry directly, no? oh thanks for reminding that i forgot to mention it here, yes we can get that, but that would misbehave in the case of virtual/remapped files. so can you actually add a comment here about why we should get the contents from source manager? ================ Comment at: clang/unittests/Tooling/HeaderTest.cpp:21 + Inputs.Code = R"cpp( + #include "headerguard.h" + #include "pragmaonce.h" ---------------- kadircet wrote: > can you also add a test case for `#import` ? this is marked as done, but i can't see any #import statements here. am i missing something? 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
