================ Comment at: include/clang/ASTMatchers/Dynamic/Parser.h:86 @@ +85,3 @@ + virtual std::vector<MatcherCompletion> + getNamedValueCompletions(llvm::ArrayRef<ArgKind> AcceptedTypes); + ---------------- Samuel Benzaquen wrote: > Peter Collingbourne wrote: > > This approach requires a bunch of common logic to be duplicated between > > individual Sema implementations. Instead, you could have a virtual function > > in this class that returns (a const reference to) a map of named values, > > which would be called by this function and getNamedValue. > > > > You might also consider changing the parser API to pass the map separately > > (along with Sema) to the parser. That way, you could also avoid > > complicating Sema too much as a result of having to allow > > completeExpression to take custom Semas. > Having this in the Sema object is following the same logic as having the > matchers. > It allows an implementation of it to do whatever it wants. It could be backed > by an in-memory std::map, be generated on demand, or by some other source. > Earlier, I added 'RegistrySema' because it was common to use that one. > I was thinking in providing a MappedValueSema, and make both of these mix-ins > that the user can add to their implementations. > > wrt duplicated logic, I added most of the required logic in the support > classes (VariantValue, etc). The implementation of this method of a backing > string->value map is just a few lines. I realize why you added this flexibility, and I sort of see the value in it, but I'm not convinced that it is necessary.
If I were designing this, I would make the std::map part of the interface to the parser, and defer the design of any additional flexibility for when it is needed, so that I'd have something concrete to base my design on. For example, it could be done incrementally on top of the std::map design by adding something to Sema that lets a client augment the contents of the map. I won't insist on you changing your current design though. http://reviews.llvm.org/D3509 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
