[PATCH] D74361: [Clang] Undef attribute for global variables

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield marked 2 inline comments as done.
JonChesterfield added a comment.

Fixed the spelling/formatting, added more tests. The C++ case would be improved by warning on `int x __attribute__((loader_uninitialised)) = 0` as there are two initializers.

The semantics for C are not what I hoped for where there are multiple definitions, one of which is via this attribute. Added a test for that. Recommendations for where to poke sema to raise an error on the second one are warmly invited.



================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7436
+  case ParsedAttr::AT_LoaderUninitialized:
+    handleLoaderUninitializedAttr(S, D, AL);
+    break;
----------------
aaron.ballman wrote:
> If you don't need any custom semantic checking, you can remove that function and instead call `handleSimpleAttribute<LoaderUninitializedAttr>(S, D, AL);`
I think this patch does need some custom semantic checking, I just haven't been able to work out how to implement it. Specifically, the attribute is an initializer, so

`int foo __attribute__((loader_uninitialised)) = some_value;`

should be a warning, as the = some_value is going to be ignored.


================
Comment at: clang/test/CodeGenCXX/attr-loader-uninitialized.cpp:14
+// CHECK: @tentative = global i32 undef
+int tentative  [[clang::loader_uninitialized]];
+
----------------
aaron.ballman wrote:
> What should happen with redeclarations? e.g., in C:
> ```
> int a;
> 
> int foo() { return a; }
> 
> int a __attribute__((loader_uninitialized));
> ```
> (This would be a useful test case to add.)
> 
> Also, I'd like to see a test case where the attributed global is an array.
Ah, yes. I was thinking about tentative definitions before changing this test to C++. Fixed the name to be less misleading.

C++ just rejects it. Multiple definitions => error. Added test to sema.

C accepts it in either order. Which I believe it should. Either one is a tentative definition, and the other provides an actual definition (of undef), or the definition (of undef) is followed by a redeclaration.

This leaves the hole that while the following is rightly rejected in C for having multiple definitions:
```int a __attr__(...);
int a = 10;```

This is erroneously accepted, with the attribute ignored:
```int a = 10;
int a __attr__(...);```






Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74361/new/

https://reviews.llvm.org/D74361