[Lldb-commits] [PATCH] D35083: [TypeSystem] Guard the global `ASTSourceMap` with a mutex

[Lang Hames via Phabricator via lldb-commits]

lhames added a comment.

I prefer the lambda syntax: Locking with an argument requires you to know the idiom (which wasn't immediately obvious to me), whereas the lambda enforces it implicitly.

We could also consider writing an atomic access adapter for this (and other types). That would be generally useful, but would also require more consideration (i.e. we shouldn't block this patch on that discussion).



================
Comment at: source/Symbol/ClangExternalASTSourceCommon.cpp:24
+template <typename FnType>
+static decltype(std::declval<FnType>()(std::declval<ASTSourceMap&>()))
+WithExclusiveSourceMap(FnType fn) {
----------------
spyffe wrote:
> lhames wrote:
> > Does std::result_of<FnType>::type work as the return type?
> > 
> > http://en.cppreference.com/w/cpp/types/result_of
> No, it doesn't.  If I change the declaration to
> ```
> template <typename FnType>
> static typename std::result_of<FnType>::type
> WithExclusiveSourceMap(FnType fn) {
> ```
> then the compiler can no longer resolve calls:
> ```
> …/lldb/source/Symbol/ClangExternalASTSourceCommon.cpp:25:1: Candidate template ignored: substitution failure [with FnType = (lambda at …/lldb/source/Symbol/ClangExternalASTSourceCommon.cpp:39:7)]: implicit instantiation of undefined template 'std::__1::result_of<(lambda at …/lldb/source/Symbol/ClangExternalASTSourceCommon.cpp:39:7)>'
> ```
Oh right, result_of only works for function types at the moment.

One minor tidy-up would be to use the 'auto' function declaration syntax so that you can use the name of the functor in the decltype expression, rather than having to declval one up. i.e.:

  static decltype(std::declval<FnType>()(std::declval<ASTSourceMap&>()))
  WithExclusiveSourceMap(FnType fn) {

becomes

  static auto WithExclusiveSourceMap(FnType fn) -> declval(fn(std::declval<ASTSourceMap&>())) {
    ...


Repository:
  rL LLVM

https://reviews.llvm.org/D35083