On Sep 17, 2009, at 2:32 PM, Douglas Gregor wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=82166&view=rev
> Log:
> Initial implementation of a code-completion interface in Clang. In
Woot!
> +++ cfe/trunk/lib/Lex/Lexer.cpp Thu Sep 17 16:32:03 2009
> @@ -1309,6 +1310,18 @@
> return true; // Have a token.
> }
>
> + if (IsEofCodeCompletion) {
> + // We're at the end of the file, but we've been asked to
> conside the
> + // end of the file to be a code-completion token. Return the
> + // code-completion token.
> + Result.startToken();
> + FormTokenWithChars(Result, CurPtr, tok::code_completion);
> +
> + // Only do the eof -> code_completion translation once.
> + IsEofCodeCompletion = false;
> + return true;
> + }
Can this be moved after the "lexing raw mode" case, so that raw mode
lexing doesn't pay the cost of an extra check?
> +++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h Thu Sep 17
> 16:32:03 2009
> + /// \brief Captures a result of code completion.
> + struct Result {
> + /// \brief Describes the kind of result generated.
> + enum ResultKind {
> + RK_Declaration = 0, //< Refers to a declaration
> + RK_Keyword, //< Refers to a keyword or symbol.
> + };
> +
> + /// \brief The kind of result stored here.
> + ResultKind Kind;
> +
> + union {
> + /// \brief When Kind == RK_Declaration, the declaration we
> are referring
> + /// to.
> + NamedDecl *Declaration;
> +
> + /// \brief When Kind == RK_Keyword, the string representing
> the keyword
> + /// or symbol's spelling.
> + const char *Keyword;
> + };
Wouldn't it be nice if PointerIntPair<> fell back to pair<void*,int>
if the pointer had no extra bits? :)
Should a Result have an optional "extra information" flag or do you
expect informational stuff to be derived from the decl in all cases?
i.e. not from sema info?
> + /// \brief A container of code-completion results.
> + class ResultSet {
The naming here is a bit strange. The "Result" of a code completion
event is a "ResultSet", and a single entry in the result is a
"Result". How about naming the classes "ResultEntry" and "Result" or
something?
> + /// \brief The actual results we have found.
> + std::vector<Result> Results;
> +
> + /// \brief A mapping from declaration names to the declarations
> that have
> + /// this name within a particular scope and their index within
> the list of
> + /// results.
> + typedef std::multimap<DeclarationName,
> + std::pair<NamedDecl *, unsigned> >
> ShadowMap;
> +
> + /// \brief A list of shadow maps, which is used to model name
> hiding at
> + /// different levels of, e.g., the inheritance hierarchy.
> + std::list<ShadowMap> ShadowMaps;
This is a very heavy-weight structure. I haven't read this in detail
to understand why you need this, but... do you really need this? :)
> +
> + public:
> + typedef std::vector<Result>::iterator iterator;
> + iterator begin() { return Results.begin(); }
> + iterator end() { return Results.end(); }
How about just exposing const_iterator instead of iterator? Do you
expect clients to want to mutate this?
> + Result *data() { return Results.empty()? 0 : &Results.front(); }
likewise, const Result*?
> + /// \brief Add a new result to this result set (if it isn't
> already in one
> + /// of the shadow maps), or replace an existing result (for,
> e.g., a
> + /// redeclaration).
> + void MaybeAddResult(Result R);
> +
> + /// \brief Enter into a new scope.
> + void EnterNewScope();
> +
> + /// \brief Exit from the current scope.
> + void ExitScope();
This style of API seems strange to me: it seems that you'd want to
pass a fully formed "ResultSet" from Sema into an implementation of
CodeCompleteConsumer, but ResultSet has a bunch of "builder" APIs.
Would it make sense to split this out into a CCBuilder thing?
> + /// \brief Process the finalized code-completion results.
> + virtual void ProcessCodeCompleteResults(Result *Results,
> + unsigned NumResults) { }
Oh, I see, Sema only gets the array of results. Given this, should
"ResultSet" be renamed "ResultBuilder" and moved out of the public
header file into a private header in the Sema directory?
> + /// \brief Code completion for a member access expression, e.g.,
> "x->" or
> + /// "x.".
> + ///
> + /// \param S is the scope in which code-completion occurs.
> + ///
> + /// \param BaseType is the type whose members are being accessed.
> + ///
> + /// \param IsArrow whether this member referenced was written
> with an
> + /// arrow ("->") or a period (".").
> + virtual void CodeCompleteMemberReferenceExpr(Scope *S, QualType
> BaseType,
> + bool IsArrow);
> +
> + /// \brief Code completion for a qualified-id, e.g., "std::"
> + ///
> + /// \param S the scope in which the nested-name-specifier occurs.
> + ///
> + /// \param NNS the nested-name-specifier before the code-
> completion location.
> + ///
> + /// \param EnteringContext whether the parser will be entering
> the scope of
> + /// the qualified-id.
> + virtual void CodeCompleteQualifiedId(Scope *S,
> NestedNameSpecifier *NNS,
> + bool EnteringContext);
> + //@}
Why do these need to be in the public CodeCompleteConsumer interface?
Doesn't Sema handle all the details of this? Are implementations of
consumers expected to implement these?
> +++ cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp Thu Sep 17 16:32:03
> 2009
> @@ -0,0 +1,314 @@
> +//===---- CodeCompleteConsumer.h - Code Completion Interface ----*-
> C++ -*-===//
Please update this line
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open
> Source
> +// License. See LICENSE.TXT for details.
> +//
> +//
> =
> =
> =
> ----------------------------------------------------------------------=
> ==//
> +//
> +// This file implements the CodeCompleteConsumer class.
> +//
> +//
> =
> =
> =
> ----------------------------------------------------------------------=
> ==//
> +#include "clang/Sema/CodeCompleteConsumer.h"
> +#include "clang/Lex/Preprocessor.h"
> +#include "Sema.h"
> +#include "llvm/ADT/STLExtras.h"
> +#include "llvm/Support/Compiler.h"
No need to use VISIBILITY_HIDDEN for stuff in anonymous namespaces
anymore. We've decided gcc 4.0 doesn't matter for performance anymore.
> +#include "llvm/Support/raw_ostream.h"
> +#include <algorithm>
> +#include <string.h>
<cstring>
Overall, very nice!
-Chris
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
