eandrews added inline comments.
================
Comment at: lib/CodeGen/CodeGenModule.cpp:2434
+ // Emit section information for extern variables.
+ if (D->hasExternalStorage() && !D->isThisDeclarationADefinition()) {
+ if (const SectionAttr *SA = D->getAttr<SectionAttr>())
----------------
efriedma wrote:
> eandrews wrote:
> > efriedma wrote:
> > > Why do you specifically check "D->hasExternalStorage() &&
> > > !D->isThisDeclarationADefinition()", instead of just setting the section
> > > unconditionally?
> > I noticed that you enter GetOrCreateLLVMGlobal( ) whenever the extern
> > variable is declared as well as when it is defined. The flow of the program
> > is different in each case. When the variable is defined, it also enters
> > EmitGlobalVarDefinition( ). There is existing code handling section
> > information here. I added the check in GetOrCreateLLVMGlobal( ) so the
> > block gets skipped for variable definition, since its already handled
> > elsewhere.
> I would rather just call setSection unconditionally here, as opposed to
> trying to guess whether the global will eventually be defined.
>
> I'm also sort of concerned the behavior here could be weird if a section
> attribute is added on a redeclaration (e.g. what happens if you write `extern
> int x; int y = &x; extern int x __attribute((section("foo")));`)... maybe we
> should emit a warning?
@efriedma I modified the patch to emit a warning in the scenario you
mentioned. A warning is also emitted for the following -
```extern int x; int *y=&x; int x __attribute__((section("foo")));
```
I thought it made sense to emit the warning for the latter as well. Should I
restrict the warning to just redeclarations (without definition) instead?
https://reviews.llvm.org/D36487
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
