<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>Sorry for the long delay in getting back to you!</div><br><div><div>On Oct 31, 2013, at 9:36 AM, Erik Verbruggen <<a href="mailto:erikjv@me.com">erikjv@me.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br>On 31 Oct 2013, at 0:57, Argyrios Kyrtzidis <<a href="mailto:kyrtzidis@apple.com">kyrtzidis@apple.com</a>> wrote:<br><br><blockquote type="cite">Hi Erik, sorry for the delay!<br><br>On Oct 12, 2013, at 4:25 AM, Erik Verbruggen <<a href="mailto:erikjv@me.com">erikjv@me.com</a>> wrote:<br><br><blockquote type="cite">It has been some time since the last time I did a patch...<br><br>Record ranges skipped by the preprocessor and expose them with libclang.<br></blockquote><br>Cool!<br><br><blockquote type="cite"><br>This requires the use of a detailed preprocessing record. Also bumbed the cindex minor version to reflect adding new functionality (and to be able to detect that during built-time).<br><br>The patch is against r192531, which has the same amount of failures for me on MacOS as trunk (two, both with OpenCL).<br><br>Feedback please! :)<br></blockquote><br>Some nitpicks:<br><br>+/**<br>+ * \brief Retrieve all ranges that were skipped by the preprocessor.<br>+ */<br>+CINDEX_LINKAGE CXSkippedRanges *clang_getSkippedRanges(CXTranslationUnit tu);<br><br>Should we have a function to get all skipped ranges, and another to get the skipped ranges of a particular file ? IMO the latter is much more useful.<br></blockquote><br>You're right of course. So for API design: add a const char *filename as second parameter?<br></div></blockquote><div><br></div><div>I suggest a "CXFile file" parameter.</div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br><blockquote type="cite">+<br>+ /// \brief Retrieve all ranges that got skipped while preprocessing.<br>+ const std::vector<SourceRange> &getSkippedRanges() const {<br>+ return SkippedRanges;<br>+ }<br><br>Please use ArrayRef here.<br><br>Also we would need to serialize the skipped ranges to the preprocessing record of a PCH as well but this can be done later.<br></blockquote><br>Do we need that? I mean, my use-case is to grey-out the skipped ranges/lines in an IDE. When a file is opened that's part of a PCH, it will probably need a reparse anyway... But I might be missing something here.<br></div></blockquote><div><br></div><div>If you are just browsing the header we can get all the information from the PCH/precompiled preamble, we don't need to reparse.</div><div>Or do you mean something else ? I don't understand why we need to reparse anyway..</div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br><blockquote type="cite">+// RUN: env CINDEXTEST_SHOW_SKIPPED_RANGES=1 c-index-test -test-annotate-tokens=%s:1:1:16:1 %s | FileCheck %s<br>+// CHECK: Skipping: [5:2 - 6:7]<br>+// CHECK: Skipping: [8:2 - 12:7]<br>+// CHECK: Skipping: [14:2 - 20:7]<br><br>Should we just unconditionally (I mean without the CINDEXTEST_SHOW_SKIPPED_RANGES env variable) show the skipped ranges for the '-test-annotate-tokens' option (and update any test if it breaks) ?<br></blockquote><br>Will do that.<br><br><blockquote type="cite">Also the test is not also testing if tokens are properly annotated/not-annotated, could you add some CHECK/CHECK-NOT lines for some of the tokens ?<br></blockquote><br>Sorry, I don't understand your question. Can you give an example?<br></div></blockquote><div><br></div><div>Never mind, it's not important.</div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br>I just noticed that my rebase removed the CINDEX_VERSION_MINOR bump. I'll add it back in, if that's okay?<br></div></blockquote><div><br></div><div>Yes, certainly.</div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br>Thanks!<br>-- Erik.<br><br><br>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a></div></blockquote></div><br></body></html>