<div dir="ltr">Thanks for taking the time to look at this!<br><br>The ctags database seemed like a good way to get a simple backend up and running. I agree that it is not an ideal tool for C++. Of the various efforts going on to build this sort of database, is there one that is generally preferred by the community? For example, there exists cldoc <<a href="http://jessevdk.github.io/cldoc/">http://jessevdk.github.io/cldoc/</a>>, which claims to serialize to an XML-based representation. Maybe going from this representation to something like JSON (since there's already parsing code for this checked in to support compilation databases) would not be too bad.<br>
<br>I thought I'd normalized the function names to the surrounding Clang code, but it looks like I was following an out-of-date standard. (Maybe someone someday will work out a good way of getting clang-format to understand the morphology and case rules for identifiers.)<br>
<br>As to whether this should be part of libclang or a separate tool, I will need to learn more about the former before I can think of a good answer! I regret that I have significantly less time to work on this now than when I was building it as a summer project, so any large refactoring or split work may be blocked for a while, but I am still happy to receive comments.<br>
<br>Thank you again for your time.<br><br> Luke<br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 11, 2013 at 5:52 PM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="im">On Fri, Oct 11, 2013 at 10:00 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div class="im">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi Luke,<div><br></div><div>I think in general this makes sense. One problem is that it's a really large change, so be prepared for it to require a longer time for the review - if there's any way to split it up into "manageable" chunks, that provide incremental functionality (note that splitting up vertically will not help at all), that would be super useful...</div>
<div><br></div><div>One concern I have is the ctags database. I haven't seen useful ctags tools for C++ yet, and I'd vote for rather implementing something simple that works well for C++ on our own than importing a ctags parser. That said, I've not yet looked at the code in detail, as the change is large enough that it doesn't load in phabricator for me (I filed an upstream bug ;)</div>
</div></blockquote><div><br></div></div><div>A few more high level comments:</div><div>- method names in clang should be camelCase() (<a href="http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly" target="_blank">http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly</a>)</div>
<div>- dumping a "public domain" implementation of a ctags parser will not fly (according to Chandler) - if we want to support ctags, we'll probably reimplement it to make sure it's provided with a LLVM compatible license</div>
<div>- why do we want a tool instead of providing an extension point in libclang to hook this directly into the parser's recovery?</div><div><br></div><div>Cheers,</div><div>/Manuel</div><div><div class="h5"><div><br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">
<div><br></div><div>Cheers,</div><div>/Manuel</div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div>On Tue, Sep 10, 2013 at 11:49 AM, Luke Zarko <span dir="ltr"><<a href="mailto:lukezarko@gmail.com" target="_blank">lukezarko@gmail.com</a>></span> wrote:<br>
</div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div><div dir="ltr">Crossposting from cfe-commits, as suggested.<br>
<br>The code is up at <a href="http://llvm-reviews.chandlerc.com/D1538" target="_blank">http://llvm-reviews.chandlerc.com/D1538</a> .<br>
<br>This is a tool that uses an external database, like a ctags file, to suggest missing includes to fix a broken translation unit. It can produce diagnostics like:<br>
<br> TESTFILE:6:9: note: Inferring that M names a namespace.<br> using ::M::X;<br> ^<br> TESTFILE:6:12: note: For M::X, include m_x_empty_class.h.<br> using ::M::X;<br> ^<br><br>or:<br><br> TESTFILE:6:1: note: For M::Q::Y, include m_q_y_empty_class.h.<br>
Y a;<br> ^<br> TESTFILE:6:1: note: Alternately, for N::Q::Y, include n_q_y_empty_class.h.<br><br>It works by looping for a bounded number of times. On each iteration, the broken TU is parsed and passed through semantic analysis. When typos are encountered or when an incomplete type is detected where a complete type is required, the database is consulted to try and find a header with an appropriate declaration or definition. The header is inserted into the TU (currently using a fixed formatting rule).<br>
<br>The primary components of the tool and their roles are:<br><br>SemaSearchPlugin: An interface to a semantic database providing name-to-definition-file queries and limited name-to-kind (here, namespace) queries. It's useful to know if some fully-qualified name points to a namespace because it allows the tool to 'fake out' missing namespace declarations without confusing them with static members (Foo::Bar vs ClassFoo::InnerClass::Bar). The tool does this when a typo correction hook isn't able to find a name during the Sema::LookupNestedNameSpecifierName LookupKind.<br>
<br>CtagsSemaSearchPlugin: A specialization for SemaSearchPlugin that works over Exuberant Ctags files. (Parsing these files is handled by the readtags library.)<br><br>FileTracker: Keeps track of data associated with a particular TU being fixed across tool iterations. Holds a MemoryBuffer that contains the most recent version of the file that was rewritten (if rewriting occurred).<br>
<br>FyiAction: Hooks MaybeDiagnoseMissingCompleteType and CorrectTypo calls from Sema.<br><br>In the former case, since a complete type has a fully qualified name, a simple query can be made on the SemaSearchPlugin (eg, if we see that Foo::Bar is incomplete in a context where a complete type is necessary, we can query for that fully-qualified name). The database may contain multiple entries for a definition site; ranking these is the plugin's duty. The highest-ranked suggestion is used to add an include, while other suggestions may be displayed as diagnostic notes. (The ctags plugin simply returns an unordered vector.)<br>
<br>In the latter case, the mapping between (un)qualified name and fully-qualified name is ambiguous. The tool searches DeclContexts from innermost to outermost, appending the typo's NestedNameSpecifier as appropriate (and taking care to first expand namespace aliases). If it fails a lookup in the database, it will then (as a heuristic) look in the namespaces of available 'using' declarations. Finally, it will try simple unqualified lookup.<br>
<br>FyiDiagnosticHooks: Registers custom diagnostics, acts as a diagnostic sink to collect diagnostics (so that intermediate steps in the outer tool loop don't litter the terminal), and flags some warnings and notes as errors to force the tool to spin around again--for example, if the compiler generates a diag::warn_delete_incomplete, the tool should treat this as seriously as any other incomplete-type error.<br>
<br>FyiPreprocessorHooks: Listens for file-entered and file-left events, watches for inclusion directives, and keeps track of insertion positions in target TUs. The rewrite and header-equality mechanisms currently assume a particular programming style (system first, then qualified-from-project-root) and that the missing headers will be quoted.<br>
<br>FyiActionFactory: Manages the map from filenames to FileTrackers and the global iteration count. FyiActionFactory can save out changes to files (optionally with new suffixes or after moving the old file to a backup).<br>
<br>FindYourIncludes: Contains the tool's entry point.<br><br>More information on the various components is available in Doxygen comments throughout the source code.<br></div>
<br></div></div><div>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
<br></div></blockquote></div><br></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>