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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
