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

[C++] Remove libuuid dependency #3787

Merged
merged 1 commit into from Aug 6, 2022
Merged

[C++] Remove libuuid dependency #3787

merged 1 commit into from Aug 6, 2022

Conversation

Technius
Copy link
Contributor

@Technius Technius commented Jul 15, 2022

This PR removes the libuuid dependency from the C++ runtime, since the libuuid headers are not referenced anywhere. Note that libuuid is only used as a dependency on Linux.

Context: I'm working on a C++ project that uses Nix to manage dependencies and was trying to use the CMake ANTLR config file (as generated by -DANTLR_INSTALL=on), but I was getting linker errors in my project as 1) libuuid is not checked for in the exported CMake config file; and 2) an unnecessary -luuid was added to the antlr-runtime target's interface link libraries.

libuuid and its headers are not referenced anywhere, so remove it.

Signed-off-by: Bryan Tan <bryantan@technius.net>
@parrt
Copy link
Member

parrt commented Aug 4, 2022

Makes me a bit nervous... @mike-lischke or @jcking might have an opinion.

@mike-lischke
Copy link
Member

Won't removing the lib cause linker errors for others because of missing symbols? How can that work without the linked uuid lib?

@Technius
Copy link
Contributor Author

Technius commented Aug 5, 2022

As far as I can tell, there is no #include <uuid/uuid.h> in the runtime files or any generated C++ files, so I don't believe that a symbol referencing libuuid will be contained in the antlr C++ runtime object files. Perhaps I'm missing something? In any case, I tried the following experiment: I ran a fresh ubuntu:latest docker image, installed libantlr4-runtime4.9, and then ran nm -D $(find /usr/lib -name libantlr4-runtime.so.4.9). The symbol list did not contain anything beginning with uuid_.

@parrt
Copy link
Member

parrt commented Aug 5, 2022

Oh! @mike-lischke Isn't this related to something we removed recently in the ATN serialization? We used to have a UUID in there but it has been removed from all target.

@mike-lischke
Copy link
Member

Yes, that's what I thought too. I did a quick search and cannot find any reference to uuid. So it looks like the lib reference is just an artifact and can be removed. IIRC @KvanTTT wrote the patch that removed the uuid stuff, right?

@KvanTTT
Copy link
Member

KvanTTT commented Aug 6, 2022

@parrt parrt added this to the 4.10.2 milestone Aug 6, 2022
@parrt
Copy link
Member

parrt commented Aug 6, 2022

Thanks everyone

@parrt parrt merged commit 0b8ed20 into antlr:dev Aug 6, 2022
@jcking
Copy link
Collaborator

jcking commented Aug 6, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants