labath added a comment.
In https://reviews.llvm.org/D35083#804623, @jingham wrote:
> As a general practice requiring a wrapper like this for every use of a should
> be locked object would make the code noisy and hard to read. The only error
> you would be protecting against is that somebody used the entity after the
> lock went out of scope. You could use some kind of markup to enforce this
> requirement, but you also have to be pretty sloppy to make this kind of
> error, so I'm not sure this it worth going to great lengths to protect
> against.
>
> In this limited use, I guess I don't object seriously, but don't like this as
> a general pattern.
+1. For limited use, I am fine with leaving this up to the owners discretion,
but I would object to a more widespread usage.
================
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)>'
> ```
that should probably be something like std::result_of<FnType(ASTSourceMap&)>
================
Comment at: source/Symbol/ClangExternalASTSourceCommon.cpp:55
+ });
g_TotalSizeOfMetadata += m_metadata.size();
}
----------------
Since you're worrying about this being run concurrently, it looks like you
should also protect the access to this global variable (possibly by making it
atomic).
Repository:
rL LLVM
https://reviews.llvm.org/D35083
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
