[clang] Follow-up on #embed implementation

[Fznamznon]

I merged #68620 this morning. However there are several places to improve the implementation. This issue is to track them as well as for further discussion on the matter.
To be done:

• Look close into serialization code for EmbedExpr. Right now the storage will be duplicated if two EmbedExpr referencing same data are serialized. Context ✨ [Sema, Lex, Parse] Preprocessor embed in C and C++ (and Obj-C and Obj-C++ by-proxy) #68620 (comment)
• Transform tok::annot_embed into a bunch of tokens and inject them back into stream. We might need a new kind of token that directly hold a numerical value. Context ✨ [Sema, Lex, Parse] Preprocessor embed in C and C++ (and Obj-C and Obj-C++ by-proxy) #68620 (review)
• Solve the issue that we're copying the data if a file is page-sized. Ideas ✨ [Sema, Lex, Parse] Preprocessor embed in C and C++ (and Obj-C and Obj-C++ by-proxy) #68620 (comment) .
• Maybe look more into performance of general case. Current performance diff between "fast" and "slow" case available here ✨ [Sema, Lex, Parse] Preprocessor embed in C and C++ (and Obj-C and Obj-C++ by-proxy) #68620 (comment) .

To be discussed

• Decide how to handle -d, -E options
• if _has_include/__has_embed/__has_c_attribute may appear in the limit parameter argument. Right now accepted, we may need to diagnose that.
• What the default preprocessed output format should be, what options to provide to modify it.
• Decide how to spell base64-input.

Please feel free to edit this if I got something wrong.

CC @AaronBallman @jyknight @cor3ntin @jakubjelinek

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Mariya Podchishchaeva (Fznamznon)

I merged https://github.com//pull/68620 this morning. However there are several places to improve the implementation. This issue is to track them as well as for further discussion on the matter. To be done:

• Look close into serialization code for EmbedExpr. Right now the storage will be duplicated if two EmbedExpr referencing same data are serialized. Context ✨ [Sema, Lex, Parse] Preprocessor embed in C and C++ (and Obj-C and Obj-C++ by-proxy) #68620 (comment)
• Transform tok::annot_embed into a bunch of tokens and inject them back into stream. We might need a new kind of token that directly hold a numerical value. Context ✨ [Sema, Lex, Parse] Preprocessor embed in C and C++ (and Obj-C and Obj-C++ by-proxy) #68620 (review)
• Solve the issue that we're copying the data if a file is page-sized. Ideas ✨ [Sema, Lex, Parse] Preprocessor embed in C and C++ (and Obj-C and Obj-C++ by-proxy) #68620 (comment) .
• Maybe look more into performance of general case. Current performance diff between "fast" and "slow" case available here ✨ [Sema, Lex, Parse] Preprocessor embed in C and C++ (and Obj-C and Obj-C++ by-proxy) #68620 (comment) .

To be discussed

• Decide how to handle -d<letter>, -E options
• if _has_include/__has_embed/__has_c_attribute may appear in the limit parameter argument. Right now accepted, we may need to diagnose that.
• What the default preprocessed output format should be, what options to provide to modify it.
• Decide how to spell base64-input.

Please feel free to edit this if I got something wrong.

CC @AaronBallman @jyknight @cor3ntin @jakubjelinek

[AaronBallman]

There are also some more follow-up concerns that were raised on the original PR, starting from #68620 (comment)

[Fznamznon]

I posted a reland #95802 .

[R-Goc]

Is there a plan to implement a feature test macro for this? In the C++ paper, p1967, a feature test macro, __cpp_pp_embed is proposed. Or is there another way to check if embed is present for C++ that I missed, with some has_extension or has_feature macro?

[AaronBallman]

Is there a plan to implement a feature test macro for this? In the C++ paper, p1967, a feature test macro, __cpp_pp_embed is proposed. Or is there another way to check if embed is present for C++ that I missed, with some has_extension or has_feature macro?

In C, the feature test macro is __STDC_VERSION__; the feature has not yet been adopted in C++ and so we're in a bit of a holding pattern for what feature test macro we expose there.

[jakubjelinek]

Why do you need another feature test macro?
#ifdef __has_embed
or
#ifdef __has_embed
#if __has_embed(FILE)
should be enough for both C and C++.

[Fznamznon]

I have created PR #97274 to address the second item in the TODO list (Transform tok::annot_embed into a bunch of tokens and inject them back into stream. We might need a new kind of token that directly hold a numerical value.)

[h-vetinari]

Just noticed that #embed isn't mentioned under in the C23 section of the clang 19 release notes.

[AaronBallman]

Just noticed that #embed isn't mentioned under in the C23 section of the clang 19 release notes.

Good catch, the release note seems to have been dropped.

[Fznamznon]

Just noticed that #embed isn't mentioned under in the C23 section of the clang 19 release notes.

Here it is #97997

[Fznamznon]

Found one more bug via testing against gcc tests
https://godbolt.org/z/cffPqhfob
Also there is #97274 (comment) .

[Fznamznon]

embed crash with std::initializer_list #99050 (comment) .
The crash itself can be fixed easily, but making clang accept the code seems more complicated.