Issue metadata
Sign in to add a comment
|
rewrite_to_chrome_style: wtf::MutexBase::lock is not rewritten to Lock |
||||||||||||||||||||||
Issue description
third_party/WebKit/Source/wtf/ThreadingPrimitives.h got rewritten as follows:
class WTF_EXPORT MutexBase {
WTF_MAKE_NONCOPYABLE(MutexBase); USING_FAST_MALLOC(MutexBase);
public:
~MutexBase();
void lock();
void unlock();
#if ENABLE(ASSERT)
bool Locked() { return mutex_.recursion_count_ > 0; }
#endif
Note that |locked| got rewritten to |Locked|, but |lock| and |unlocked| where not rewritten here. This is a problem, because |lock| *did* get rewritten in third_party/WebKit/Source/wtf/Locker.h:
template <typename T> class Locker final {
STACK_ALLOCATED();
WTF_MAKE_NONCOPYABLE(Locker);
public:
Locker(T& lockable) : lockable_(lockable) { lockable_.Lock(); }
~Locker() { lockable_.Unlock(); }
private:
T& lockable_;
};
Note that the repro above happens only after the following 3 CLs are applied:
- https://codereview.chromium.org/2246263002
- https://codereview.chromium.org/2274653002
- https://codereview.chromium.org/2256913002
Without the CLs above, |lock| inside Locker.h doesn't get rewritten (and so inconsistent rewriting in ThreadingPrimitives.h is not a compile problem). I am guessing that |lock| inside Locker.h is a CXXDependentScopeMemberExpr (which starts getting rewritten in https://codereview.chromium.org/2256913002).
,
Aug 23 2016
Ah ya thats for interacting with STL stuff. If this never does you could whitelist them.. if you want :P
,
Aug 23 2016
所以這個是!?
,
Aug 23 2016
Google translate suggests you're asking what this is, you can see the blocking bug for a higher level idea https://bugs.chromium.org/p/chromium/issues/detail?id=578344
,
Aug 24 2016
This is taken care of by https://codereview.chromium.org/2274653002/#ps100001, which starts to obey blacklists for unresolved / decl-less nodes. Therefore I think I'll lazily resolve this bug as a dupe of 640016 (and for now won't follow the whitelisting suggestion from #c2 - AFAICT Locker is not used with std namespace today, but the way it is written allows [AFAIK] substituting T=std::mutex).
,
Aug 24 2016
Cool SGTM |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by lukasza@chromium.org
, Aug 23 2016