AhmedKamel10 wrote: Thanks for reviewing, I fixed the inline comments and had to look deeper into the old testing code. what this patch currently does: C23 6.9.3 says a file-scope `thread_local` declaration is a definition, never a tentative one. The patch follows this in all language modes, including `_Thread_local` and` __thread`. About the tests: 1- In `attr-tls_model.c` and `aix-attr-tls_model.c`, I gave each variable its own name, because repeating the same name is now an error. 2- Three CodeGen tests failed because of the order of the globals in the IR (`thread-specifier.c`, `toc-data-attribute.c`,` toc-data-diagnostics.c`). After the change, the TLS variables are still in the output with the same types and attributes, but they now appear in source order (earlier than before). I believe this is because tentative definitions are emitted at the end of the file and definitions are emitted in place, but I haven't checked the codegen code to confirm it. I only moved the CHECK lines.
Also I checked the behavior of GCC locally and on godbolt and these are the findings: Same `_Thread_local` / `__thread` variable declared twice, no initializer: - GCC 13.3: accepted - GCC 15.2 `-std=gnu11`: accepted - GCC 15.2 (C23, the default): error - Clang with patch: error So as you can see, GCC applies the C23 rule only in C23 mode. Before C23 it accepts these declarations, while this patch rejects them in every mode, so existing code that repeats static __thread int y; could stop compiling. Should this rule apply in every language mode, or only in C23? If only in C23, I can add a language-mode check to the patch, and the test changes above may no longer be needed. I'd like your guidance before I change anything. https://github.com/llvm/llvm-project/pull/219780 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
