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

Fix path processing #190

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fmaksim74
Copy link
Contributor

Redundant check of "self" variable breaks search path processing. It create new instance of Parser and discards all paths added with "addpath" method.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 99.652% when pulling f43a275 on fmaksim74:fix-path-processing into 6ffd7a2 on starwing:master.

@starwing
Copy link
Owner

starwing commented Apr 8, 2022

this behavior is on purpose.

Someone may use require "protoc":load [[...]] to load a protobuf schema into pb module. But that is only permitted in demo usage. Because in this case the table version of schema may get stuck in memory forever. So a commit added at cb977e4 to force use proto.new() if anyone wants load schema multiple times. The parser created by protoc:load() usage will drop immediately.

@fmaksim74
Copy link
Contributor Author

I checked this case:

print(require "protoc":load([[
   message Phone {
      optional string name        = 1;
      optional int64  phonenumber = 2;
   }
   message Person {
      optional string name     = 1;
      optional int32  age      = 2;
      optional string address  = 3;
      repeated Phone  contacts = 4;
   } ]]))

it works fine.
require "protoc" returns valid pointer to Parser.

@starwing
Copy link
Owner

starwing commented Apr 8, 2022

Yes, but this example means the Phone/Person tables are anchored to Lua memory and can not collect i.e. the memory leak. and it prevent you hotfix the message (you can not load the same name of message again):

print(require "protoc":load([[
   message Phone {
      optional string name        = 1;
      optional int64  phonenumber = 2;
   }
   message Person {
      optional string name     = 1;
      optional int32  age      = 2;
      optional string address  = 3;
      repeated Phone  contacts = 4;
   } ]]))
print(require "protoc":load([[
   message Phone {
      optional string name        = 1;
      optional int64  phonenumber = 2;
   }
   message Person {
      optional string name     = 1;
      optional int32  age      = 2;
      optional string address  = 3;
      repeated Phone  contacts = 4;
   } ]]))
true    145
lua: .\protoc.lua:82: <input>:1:18: type .Phone already defined
stack traceback:
        [C]: in function 'error'
        .\protoc.lua:82: in function <.\protoc.lua:80>
        (...tail calls...)
        .\protoc.lua:630: in local 'top_parser'
        .\protoc.lua:851: in function 'protoc.parse'
        .\protoc.lua:1162: in upvalue 'do_compile'
        .\protoc.lua:1166: in function 'protoc.compile'
        .\protoc.lua:1176: in function 'protoc.load'
        tt.lua:15: in main chunk
        [C]: in ?

Copy link

sonarcloud bot commented Nov 19, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

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.

4 participants