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

Allow the same .stanfunctions file to be #included multiple times #1450

Open
WardBrian opened this issue Sep 23, 2024 · 6 comments
Open

Allow the same .stanfunctions file to be #included multiple times #1450

WardBrian opened this issue Sep 23, 2024 · 6 comments
Labels
feature New feature or request good first issue Good for newcomers parsing issues related to the parser and syntax errors typechecker

Comments

@WardBrian
Copy link
Member

This came up at StanCon as a common pain point when trying to organize groups of functions.

Suppose you have functions foo, bar, and baz living in files of the same names. foo is used by bar and by baz.

If you #include bar.stan and #include foo.stan in the same file, then you will end up with two (transitive) includes of foo.stan, and this will lead to a typechecker error for re-definition of the function foo.

Some solutions:

  • Add something like #pragma once in C++. This is the most explicit version, but it is probably my least favorite since it introduces a whole new verbiage

  • Automatically de-duplicate files that end with .stanfunctions. Note that we don't want to deduplcate arbitrary #include statements since they may not actually live in the same scope and therefore it would be okay. Therefore, restricting to .stanfunctions file extensions ensures this is only doing something where it matters. Downside: introduces a semantic difference in the language based on the file extensions used.

  • Allow duplicate function definitions if both definitions are from the same ultimate file. This would require some extra location logic in the typechecker.

@WardBrian WardBrian added good first issue Good for newcomers feature New feature or request typechecker parsing issues related to the parser and syntax errors labels Sep 23, 2024
@SteveBronder
Copy link
Contributor

Add something like #pragma once in C++. This is the most explicit version, but it is probably my least favorite since it introduces a whole new verbiage

Is there a reason we need something like #pragma once and not just directly using #pragma once directly? We could just do ifdefs for .stanfunction files

@WardBrian
Copy link
Member Author

Mostly on a design level, I think adding more stuff from the C/C++ preprocessor would be a mistake

@SteveBronder
Copy link
Contributor

Do you mean adding preprocessor stuff to the AST/MIR? What if we just had info for whether a file was a .stanfunction file? Then the c++ mir can use that for adding the pragma but any other backend could just ignore that info if they did not want to use it

@WardBrian
Copy link
Member Author

Oh I misunderstood, I was saying I didn't want to introduce #pragma once into the user-facing Stan language.

We can't use it during actual code gen because we're only generating one file, in the end. We do the preprocessing ourselves to end up with one unit, which we need for typechecking etc anyway

@nhuurre
Copy link
Collaborator

nhuurre commented Sep 25, 2024

The ideal solution would be to design a proper structure-aware import system and deprecate "C preprocessor" style textual #includes.

For example, let's say

import "otherfile.stan";

at the start of a file parses "otherfile.stan" and (effectively) prepends every block to the corresponding block in this file, without duplicating files that are imported in multiple ways. Would that cover all or majority of use cases for #include?

@WardBrian
Copy link
Member Author

I think a fair number of use cases in R packages use includes as essentially a crude macro system to duplicate a chunk of code in several places in the same file, which would obviously not be something an import could do. But overall I do like the idea of moving away from text-based preprocessing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request good first issue Good for newcomers parsing issues related to the parser and syntax errors typechecker
Projects
None yet
Development

No branches or pull requests

3 participants