Skip to content
This repository has been archived by the owner on Mar 26, 2020. It is now read-only.

Extern type defined with generic parameters incorrectly generated in C++ #313

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rdegnan
Copy link

@rdegnan rdegnan commented May 4, 2017

This commit fixes #149

@smarx
Copy link

smarx commented May 4, 2017

Automated message from Dropbox CLA bot

@rdegnan, thanks for the pull request! It looks like you haven't yet signed the Dropbox CLA. Please sign it here.

@smarx
Copy link

smarx commented May 23, 2017

Automated message from Dropbox CLA bot

@rdegnan, thanks for signing the CLA!

@artwyman
Copy link
Contributor

Hi @rdegnan. Thanks for taking a stab at this. Looks like this change breaks some existing functionality, though. When I try to build the sample app I see many errors like this:

/Users/atwyman/src/djinni/example/generated-src/cpp/item_list.hpp:13:10: error: use of class template 'std::vector'
      requires template arguments
    std::vector items;

Looks like your change is affecting the handling of template args for types like vector<> in a bad way.

For future reference, when making any changes, please make sure you can make all from the top-level of the repo (which can only run on a Mac due to the inclusion of iOS stuff). That'll re-generate and build the example apps and test suite, and run the tests. Note that you should also check in the resulting generated code. That's not a normal best-practice for generated code, but in this case, reviewing the generated code for the example and test suite is the easiest way for a reviewer like me to see the net effect of any changes to the generator.

Copy link
Contributor

@artwyman artwyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my prior comment for details. Please re-submit after resolving build/test problems, and also include generated code changes for example app and test suite.

@artwyman artwyman added the bug label Apr 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extern type defined with generic parameters incorrectly generated in C++
3 participants