search

efficient / libcuckoo

A high-performance, concurrent hash table
Other
1.61k stars 275 forks source link

Fix friend declarations that refer to templates, not classes. #103

Closed toojays closed 6 years ago

toojays commented 6 years ago

This commit addresses a couple of PW.BAD_FRIEND_DECL parse warnings thrown up by Coverity.

Apparently a friend declaration that refers to a template (as opposed to a type) is not legal C++. Who knew? Neither GCC nor MSVC seem to care. Anyhow, this commit corrects the error by specifying all the necessary template parameters to make the declarations refer to types.

dnbaker commented 6 years ago

I think Coverity is mistaken here. cuckoohashmap in this context (IE, inside the class template) refers to its own type already, just like the constructor refers to the type of the template class being instantiated, not the class template.

toojays commented 6 years ago

Hmm, that sounds plausible, in a "why didn't I think of that?" kind of way. :)

I think I'd better open a case with Coverity. Tomorrow (it's late here).

If you happen to be a language-lawyer type who can point to some standardese backing this up, that would be appreciated. Otherwise I'm just going to to with "name lookup from a member function of a nested class visits the scope of the enclosing class after examining the scope of the nested class" from http://en.cppreference.com/w/cpp/language/nested_types.

dnbaker commented 6 years ago

I'm not an expert on reading the Standard, so I can't give you an actual quote, but I believe that it's somewhere around 14.6-14.7. [I did try going through it for an answer, but it's rather dense.] Unfortunately, I don't think nested types are relevant here since it's a discussion of the use of the template name within the class template definition.

toojays commented 6 years ago

Thanks dnbaker, I think it comes down to 14.6.1 (injected-class-name is equivalent to template specialization) + 9.7 (nested class is in the scope of the enclosing class, implies the nested class can see the injected-class-name).

I was not able to create a small reproduction case for this, which is unfortunate. I have opened a support case with Coverity and pointed them at this PR anyway.