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

Adding libraries in header causes namespace pollution #171

Open
Ashvith10 opened this issue Nov 8, 2024 · 1 comment
Open

Adding libraries in header causes namespace pollution #171

Ashvith10 opened this issue Nov 8, 2024 · 1 comment

Comments

@Ashvith10
Copy link

Ashvith10 commented Nov 8, 2024

When libraries are #include-ed in header, in this context, mpc.h, it pollutes the namespace.

After reading a few articles and posts on online boards, I've been informed that such libraries should be part of mpc.c, and mpc.h should contain the bare minimum.

Since these headers have a header guard, the compiler does not complain while building, and this behavior goes unnoticed.

To explain the situation in more detail, I have a separate branch that uses GNU Autotools to build the library dynamically. Now, when I try to follow the Build Your Own Lisp tutorial, the library build just fine without including libraries like stdio, stdlib and string.

How the actual code should look like:
prompt.c

#include <mpc.h>
#include <stdio.h>
#include <stdlib.h>

#ifdef _WIN32

#include <string.h> 

static char buffer[2048];
...

How it is currently, because of namespace pollution:
prompt.c

#include <mpc.h>

#ifdef _WIN32

static char buffer[2048];
...

This is a unpredictable behavior, the context might be lost, and in a way, this #include goes beyond the scope of the source file (that is, mpc.c) it was supposed to be contained to, all the way to our source code, and potentially even other headers, if included to introduce type definitions.

As a reference, ediltine takes it to the next step by providing a public and private header, which in my opinion, can be used to isolate type definitions, and essential public functions from private functions needed for the internal working of said public function for abstraction.

What do you think of it?

@orangeduck
Copy link
Owner

Well I agree there are probably some standard library includes in mpc.h which can be moved to mpc.c. Feel free to make a pull request moving those. However some probably need to be left in the header since there are function declarations which depend on them (e.g. <stdio.h> is probably required because of the use of FILE).

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

No branches or pull requests

2 participants