jasonjack2015 / protobuf

Automatically exported from code.google.com/p/protobuf
Other
0 stars 0 forks source link

HasExtension() called on a repeated field should cause a compile error (Protobuf 2.4.1) #678

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Compile the attached Protobuf and example program.
2. Run it: ./main

What is the expected output? What do you see instead?
The output should not print any "violated" statements.
It shows: 
$ ./main
violated: A has no extension

What version of the product are you using? On what operating system?
Protobuf 2.4.1

Original issue reported on code.google.com by kaspar.f...@dreizak.com on 6 Nov 2014 at 2:01

Attachments:

GoogleCodeExporter commented 9 years ago
For the sake of completeness, here are the files contained in the ZIP:

$ cat A.proto 
message A {
    optional string a = 1;
    extensions 100 to 1000;
}

message AExt {
    message E {
        optional string e = 1;
    }
    extend A {
        repeated E ext = 100;
    }
}

$ cat main.cc 
// $ protoc --version
// libprotoc 2.4.1
// $ c++ -L/opt/brew/lib -I/opt/brew/include/ main.cc A.pb.cc -lprotobuf -o main
// $ c++ --version
// Apple LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn)
// Target: x86_64-apple-darwin13.4.0
// Thread model: posix

#include "A.pb.h"
#include <iostream>
#include <google/protobuf/text_format.h>

void expectTrue(bool e, const std::string& msg) {
    if (!e) {
        std::cout << "violated: " << msg << std::endl;
    }
}

int main() {
    std::cout << "hello" << std::endl;

    // Clearing a message extension clears it but reports as still present
    {
        A a;
        a.AddExtension(AExt::ext)->set_e("e");
        expectTrue(a.HasExtension(AExt::ext), "A has extension");
        expectTrue(a.ExtensionSize(AExt::ext) == 1, "A's extension has size 1");
        a.ClearExtension(AExt::ext);
        expectTrue(a.ExtensionSize(AExt::ext) == 0, "A's extension has size 0");
        expectTrue(!a.HasExtension(AExt::ext), "A has no extension"); // FAILS
    }

    return 0;
}

Original comment by kaspar.f...@dreizak.com on 6 Nov 2014 at 2:02

GoogleCodeExporter commented 9 years ago
HasExtension() is for optional fields and cannot be used upon a repeated field. 
Likewise ExtensionSize() can be used against a repeated field only. In your 
example, the line that currently is failing should really cause a compile 
error. we actually already has a DCHECK to prevent this kind of user error but 
seems it's not triggered:
https://github.com/google/protobuf/blob/master/src/google/protobuf/extension_set
.cc#L194

Original comment by xiaof...@google.com on 6 Nov 2014 at 5:32

GoogleCodeExporter commented 9 years ago
That makes sense – and is consistent with the other uses of "has" in the 
API. Thanks for the response.

Original comment by kaspar.f...@dreizak.com on 6 Nov 2014 at 5:41