couchbase/fleece

strange usage of __attribute__((__pure__))

Dushistov opened this issue · 6 comments

During compilation I got bunch of warnings about «pure» attribute on function returning «void».
For example:

FLPURE inline void swapLittle(uint16_t &n) {n = _encLittle16(n);}

this looks very strange, pure means (from gcc manual)

The pure attribute prohibits a function from modifying the state of the program that is observable by means other than inspecting the function’s return value. However, functions declared with the pure attribute can safely read any non-volatile objects, and modify the value of objects in a way that does not affect their return value or the observable state of the program.

while this function clearly have observable effect on the state of program other then return value, it modify memory via passed reference.

snej commented

You must be on the dev branch; brave! I just pushed the commit with that in it; I'll fix the incorrect declarations.

snej commented

Oh wait, those declarations weren't just on dev, they were on the master branch.

snej commented

Fixed in c0d762a.

@snej

It is not completely fixed, I still see usage of not const references and void as return value:

In file included from fleece/Fleece/Tree/MutableHashTree.cc:12:
fleece/Fleece/Tree/MutableNode.hh:36:67: warning: 'pure' attribute on function returning 'void' [-Wattributes]
   36 |         FLPURE static void encodeOffset(offset_t &o, size_t curPos) {
      |                                                                   ^
In file included from fleece/Fleece/Tree/MutableHashTree.cc:12:
fleece/Fleece/Tree/MutableNode.hh:36:67: warning: 'pure' attribute on function returning 'void' [-Wattributes]
   36 |         FLPURE static void encodeOffset(offset_t &o, size_t curPos) {
      |                                                                   ^
In file included from fleece/Fleece/Tree/NodeRef.cc:20:
fleece/Fleece/Tree/MutableNode.hh:36:67: warning: 'pure' attribute on function returning 'void' [-Wattributes]
   36 |         FLPURE static void encodeOffset(offset_t &o, size_t curPos) {
      |                                                                   ^
In file included from fleece/Fleece/Tree/NodeRef.cc:20:
fleece/Fleece/Tree/MutableNode.hh:36:67: warning: 'pure' attribute on function returning 'void' [-Wattributes]
   36 |         FLPURE static void encodeOffset(offset_t &o, size_t curPos) {
      |                                                                   ^

@snej

And why actually pure and not const,
for example consider such example:

#include <stdio.h>
#include <stdint.h>

uint16_t encLittle16(uint16_t v) __attribute__ ((pure));

int main(int argc, char *argv[])
{
	const uint16_t the_same = argc;
	printf("result: %d\n", encLittle16(the_same));
	printf("result2: %d\n", encLittle16(the_same));
}

If I compile this code with clang++ -S -c -ggdb -Ofast test.cpp,
I see two callq encLittle16,
because of pure can use global state and printf can change this global state.

But if I mark it as const:

uint16_t encLittle16(uint16_t v) __attribute__ ((const));

there is only one callq encLittle16 in assembly,
may be better mark functions that not depend on global variables and get arguments by value as const, it should be much better?

snej commented

Crap, I thought those were the only void+pure functions. Thanks for catching the others.

I haven't considered the __const__ attribute yet, just __pure__ because it's more commonly useable. But you're right that the endian stuff is const-able.