[PATCH] D93102: [Clang][Sema] Detect section type conflicts between functions and variables

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3048
     D->addAttr(NewAttr);
+    if (auto FD = dyn_cast<FunctionDecl>(D))
+      if (auto SA = dyn_cast<SectionAttr>(NewAttr))
----------------
Does this need to be limited to function declarations? It seems to me that this should also be used on say ObjCMethod declarations or global variables as well, right?

If so, then `Sema::UnifySection()` may need to be updated as well as it currently accepts a `DeclaratorDecl` but some of these constructs (like `ObjcMethodDecl` and `ObjCPropertyDecl`) are `NamedDecl`s and not declarators.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3049
+    if (auto FD = dyn_cast<FunctionDecl>(D))
+      if (auto SA = dyn_cast<SectionAttr>(NewAttr))
+        S.UnifySection(SA->getName(),
----------------
`NewAttr` is already declared as being a `SectionAttr`, so this isn't needed, you can use `NewAttr->getName()` below instead.


================
Comment at: clang/test/Sema/attr-section.c:42
+void test4(void) {}
+int rw __attribute__((section("seg3,sec3"))) = 10;       // expected-error {{'rw' causes a section type conflict with 'test4'}}
----------------
Some other tests that may be interesting:
```
const int whatever1 __attribute__((section("seg4, sec4"))) = 10;
void test5(void) __attribute__((section("seg4, sec4")));
void test5(void) {}

void test6(void);
const int whatever2 __attribute__((section("seg5, sec5"))) = 10;
void test6(void) __attribute__((section("seg5, sec5")));
void test6(void) {}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93102