Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IAR ARM C++ Compiler V8.42.1.236 error from TableKeyComparator::operator= because vector_downward::operator= is deleted #6036

Closed
jdess opened this issue Jul 20, 2020 · 4 comments · Fixed by conan-io/conan-center-index#25607

Comments

@jdess
Copy link
Contributor

jdess commented Jul 20, 2020

Hello,

this has been observed with an older version of flatbuffers but is still valid in the version of flatbuffers.h from the (currently most recent) git commit #14baf45c90a076d405e75cfc41874ffff862fb72.

When trying to compile a file which includes flatbuffers.h with the IAR C++ compiler from the Embedded Workbench (version 8.42.1.236) on Windows, I get the following error during the compilation:

buf_ = other.buf_;
     ^
flatbuffers\include\flatbuffers\flatbuffers.h(1875,12): error [Pe1776]: function "flatbuffers::vector_downward::operator=(flatbuffers::vector_downward const &)" (declared at line 1002) cannot be referenced -- it is a deleted function

Checking flatbuffers::vector_downward::operator=(flatbuffers::vector_downward const &) confirms that this is a deleted function (from flatbuffers\include\flatbuffers\flatbuffers.h):

 private:
  // You shouldn't really be copying instances of this class.
  FLATBUFFERS_DELETE_FUNC(vector_downward(const vector_downward &))
  FLATBUFFERS_DELETE_FUNC(vector_downward &operator=(const vector_downward &))

A simple fix seems to use the same pattern and which also deletes the assignment operator from TableKeyComparator. Since the assignment operator is already private, this seems to be a fairly trivial and low-risk fix:

   private:
// >>> begin modification
//     `vector_downward::operator=` has been deleted, `buf_ = other.buf_;` below does not compile.
//     Also delete the assignment operator for this class as a work around:
#if 1
    FLATBUFFERS_DELETE_FUNC(TableKeyComparator &operator=(const TableKeyComparator &other));
#else
    // This was the original code:
    TableKeyComparator &operator=(const TableKeyComparator &other) {
      buf_ = other.buf_;
      return *this;
    }
#endif
// <<< end modification
  };
  /// @endcond
@aardappel
Copy link
Collaborator

I'm curious that this problem has never come up before, since that assignment statement is indeed problematic.

Agree that it would be best to delete the assignment operator. Why is it there, though? If noone is using it (if it passes CI), we can delete it.

Alternatively, if there are users, simply make buf_ a pointer instead.

@jdess
Copy link
Contributor Author

jdess commented Jul 21, 2020

My guess is that the IAR compiler has a different approach to dealing with templates than some of the other compilers (e.g. Microsoft, GNU). Since TableKeyComparator is a template structure, it may well be that most of the other compilers only look at the definition if the function gets called and instantiated for that template. IAR seems to always look at the code, even if the template (or one of its functions) does not get instantiated because there are no references to it in the code.

I also don't see why the assignment operator is defined at all. Since it is declared private and TableKeyComparator does not reference it at all, it is not accessible to anyone. Deleting it seems the more appropriate solution.

Converting buf_ into a pointer would definitely make the assignment operator work. However, then we'd need to make sure that there are no references to buf_ anywhere else in the code. Changing the publicly accessible buf_ from a reference to a pointer would be a breaking change to the API of the TableKeyComparator structure.

I'm not sure how to check the CI with the modification. If that helps, I can create a pull request and assume that this will then get pulled into the CI process before the PR would be accepted. Please let me know if you want me to create a pull request (and whether to go for the buf_ to pointer or delete solution).

@aardappel
Copy link
Collaborator

Yes, just make a PR that makes it into a deleted function. CI will run automatically. If it passes, we'll merge.

It's possible someone will come out of the woodwork saying they relied on this function, if so, we can change it again then.

@jdess
Copy link
Contributor Author

jdess commented Jul 24, 2020

Sounds good, I'll go ahead with the PR

aardappel pushed a commit that referenced this issue Jul 24, 2020
)

* [C++] Fix compiler error from deleted assignment operator (#6036)

The assignment operator of the `buf_` member is deleted, we cannot call it from the assignment operator of the `TableKeyComparator` struct.

=> Also delete the assignment operator of the `TableKeyComparator` struct (already private anyhow).

* [C++] Fix compiler error from deleted assignment operator (#6036) - fix extraneous semicolon

The assignment operator of the `buf_` member is deleted, we cannot call it from the assignment operator of the `TableKeyComparator` struct.

=> Also delete the assignment operator of the `TableKeyComparator` struct (already private anyhow).
@jdess jdess closed this as completed Jul 24, 2020
ivannp pushed a commit to ivannp/flatbuffers that referenced this issue Oct 2, 2020
… (google#6047)

* [C++] Fix compiler error from deleted assignment operator (google#6036)

The assignment operator of the `buf_` member is deleted, we cannot call it from the assignment operator of the `TableKeyComparator` struct.

=> Also delete the assignment operator of the `TableKeyComparator` struct (already private anyhow).

* [C++] Fix compiler error from deleted assignment operator (google#6036) - fix extraneous semicolon

The assignment operator of the `buf_` member is deleted, we cannot call it from the assignment operator of the `TableKeyComparator` struct.

=> Also delete the assignment operator of the `TableKeyComparator` struct (already private anyhow).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants