Skip to content

Added annotations to individual fields in a struct #324#346

Closed
aiverson wants to merge 8 commits intoterralang:developfrom
aiverson:develop
Closed

Added annotations to individual fields in a struct #324#346
aiverson wants to merge 8 commits intoterralang:developfrom
aiverson:develop

Conversation

@aiverson
Copy link
Copy Markdown
Contributor

Initial implementation of #324
Multiple comma separated annotations are not yet supported.

@aiverson
Copy link
Copy Markdown
Contributor Author

Does anyone know what is broken on AppVeyor here?

@elliottslaughter
Copy link
Copy Markdown
Member

I'm not sure why AppVeyor is failing, but I'd recommend looking at the changes to genclangpaths.lua/geninternalizedfiles.lua since those are involved in generating header files that will be imported into C code. (By the way, why are these changes required?) I suppose it could also be in lparser.cpp, but I'm having a hard time seeing how that change is related. In theory, it should be impossible for changes to the other Lua/Terra files to result in compile errors like this.

@aiverson
Copy link
Copy Markdown
Contributor Author

aiverson commented Feb 12, 2019

The change in geninternalizedfile.lua makes it not syntax error when it doesn't have any lua files to internalize.

The change in genclangpaths.lua makes it so that it can compile when the clang path has a space in it, for instance "C:\Program Files\Clang\clang.exe"

I encountered both of these problems while trying to build from source on windows, but eventually gave up on trying to get my windows system configured to build it.

Neither of them are necessary for the change I implemented, but they are improvements to the stability of the build process, I think.

@elliottslaughter
Copy link
Copy Markdown
Member

Maybe we should split those into a different PR? They seem like reasonable changes, but for this specific PR it'll be easier to debug if there's less to look at.

@aiverson
Copy link
Copy Markdown
Contributor Author

Sure, I'll revert that commit and try again.

@aiverson
Copy link
Copy Markdown
Contributor Author

That didn't magically fix it.

@elliottslaughter
Copy link
Copy Markdown
Member

Oh, please set the PR to be based on master. develop is in a strange state and I have no idea if it builds at all in AppVeyor.

@aiverson
Copy link
Copy Markdown
Contributor Author

Oh. That could be the problem. I'll put it on master.

@aiverson
Copy link
Copy Markdown
Contributor Author

Is there any chance of putting it in a good state soon?

@elliottslaughter
Copy link
Copy Markdown
Member

I don't think we're ever going to merge develop in its entirety. The parts that have been useful we've been pulling piecemeal into master. Other parts we've taken a fairly different approach on (PUC Lua support) which results in substantially less churn vs what had been done on develop. There are still some parts that would be nice to have (LLVM 3.8 debug symbols), but I haven't put in the effort to extract them.

@elliottslaughter
Copy link
Copy Markdown
Member

Closing since this is superseded by #347 (if I understand correctly---please let me know if I'm misunderstanding).

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 this pull request may close these issues.

2 participants