project-oak / oak

Meaningful control of data in distributed systems.
Apache License 2.0
1.32k stars 113 forks source link

'SenderContext' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator #4635

Open tiziano88 opened 10 months ago

tiziano88 commented 10 months ago

From clang-tidy:

/home/tzn/.cache/bazel/_bazel_tzn/dab993785bc8bd52220ddfe5340ed840/sandbox/linux-sandbox/5480/execroot/oak/cc/crypto/hpke/sender_context.h:33:7: error: class 'SenderContext' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions,-warnings-as-errors]

@ipetr0v could you take a quick look and fix that?

ipetr0v commented 10 months ago

Is this error produced by an internal clang-tidy or by the one added in https://github.com/project-oak/oak/pull/4634 ?

I'm trying to run bazel build --config=clang-tidy //cc/... locally and I don't get this error

tiziano88 commented 10 months ago

To reproduce you can add cppcoreguidelines-special-member-functions to the list of checks in https://github.com/project-oak/oak/blob/main/.clang-tidy and then run just clang-tidy

ipetr0v commented 10 months ago

clang-tidy wants me to use reinterpret_cast here: https://github.com/project-oak/oak/blob/1ef0a9d292ed8fa600932c83dd09aca1b75368e2/cc/crypto/hpke/jni/context_jni.cc#L36-L37

But then says

error: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr,-warnings-as-errors]

And we have to cast jlong into a pointer because of JNI

ipetr0v commented 10 months ago

Currently had to add -performance-no-int-to-ptr to https://github.com/project-oak/oak/blob/main/.clang-tidy

ipetr0v commented 10 months ago

Also about the error: consider replacing 'long' with 'int64' [google-runtime-int,-warnings-as-errors]. I'm not sure whether it's fine to do on Android JNI (needs additional double-checking). https://github.com/project-oak/oak/blob/1a9582119e4458667d794914f6f41ebb93090098/cc/crypto/hpke/jni/hpke_jni.cc#L53-L55

cc @conradgrobler @k-naliuka

tiziano88 commented 10 months ago

FYI I don't think this is urgent or is blocking anything, just something that should be cleaned up at some point.