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

__builtin_FILE() and __FILE__ have different strings #42

Open
mcspr opened this issue Dec 29, 2022 · 5 comments
Open

__builtin_FILE() and __FILE__ have different strings #42

mcspr opened this issue Dec 29, 2022 · 5 comments

Comments

@mcspr
Copy link

mcspr commented Dec 29, 2022

__FILE__ has a short one
https://github.com/earlephilhower/esp-quick-toolchain/blob/master/patches/gcc10.3/gcc-file-shortname.patch

Builtin gives the long one (even in the current master?)
https://github.com/gcc-mirror/gcc/blob/9b111debbfb79a0aa01091c8b1936b05a11ffcf9/gcc/builtins.cc#L9519-L9534
https://github.com/gcc-mirror/gcc/blob/9b111debbfb79a0aa01091c8b1936b05a11ffcf9/libcpp/macro.cc#L544-L576

Minor issue, but the only thing it affects is std::source_location (c++20) aka std::experimental::source_location (before c++20)
https://en.cppreference.com/w/cpp/experimental/source_location
https://en.cppreference.com/w/cpp/source_location

@earlephilhower
Copy link
Owner

What is the problem, and what do you think a solution would be? __builtin_FILE() to be shortened as well?

We shortened the file version to minimize the IRAM/flash usage (some of our templates have names that expand to 100 chars, plus space for the full user path to the install dir)

@mcspr
Copy link
Author

mcspr commented Dec 29, 2022

Just to note it as a difference, tbh it was a surprise to me that it does that. I'd prefer not solving it :)

We shortened the file version to minimize the IRAM/flash usage (some of our templates have names that expand to 100 chars, plus space for the full user path to the install dir)

(brb searching the history)
...was it related to heap / umm debug? I don't really see anything else using it for esp8266
also, pretty sure we could write a constexpr func wrapping __FILE__ and re-positioning it to the last / instead of hard-patch

@earlephilhower
Copy link
Owner

assert is in a few spots, and that macro has __FILE__ as part of the dump string. IIRC the DataSource had several instances and it added up fast. There were other spots, too. You could use NDEBUG to throw out asserts, but that's not the default option and there are good reasons to crash the machine when the asserts fail.

I don't think you can wrap __FILE__ w/o GCC code changes because it's synthetic and only exists in the preprocessor code, not any header files. But you might rewrite the assert macros (again, though, you're changing the GCC build at that point).

If there's a problem I think we can do the same patch in the C++ libs. There are lots of functional correctness patches already, two more won't make any difference. :)

@mcspr
Copy link
Author

mcspr commented Dec 29, 2022

Maybe it should do this instead?
https://reproducible-builds.org/docs/build-path/

~> g++ -Os -ffile-prefix-map=(pwd)=/foo (realpath a.cpp)
~> ./a.out
/foo/a.cpp
a.cpp

And yes, I meant the 2nd one - at specific call sites make __FILE__ into basename by wrapping it with a helper func.
Maybe only for internal use, per esp8266/Arduino#6491
inline (theoretically) works for C same as constexpr

constexpr
const char* __basename__(const char* ptr) {
...
    return ptr;
} 
...
foo(__basename__(__FILE__));

-> https://godbolt.org/z/WKr1v7Pvn (it escapes me why the asm string still has the /app/)

@earlephilhower
Copy link
Owner

I believe there were issues with the file-prefix-map when tried a couple years earlier. Either it was unsupported or the platform.txt infrastructure to make it work was spaghetti code.

For now, since it's not hurting anything, I say we can just leave this here and if it crops up sometime later we can dig deeper.

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