[PATCH] D45884: [Sema] Fix parsing of anonymous union in language linkage specification
Richard Smith - zygoloid via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 23 17:31:12 PDT 2018
rsmith added inline comments.
================
Comment at: Sema/SemaDecl.cpp:4654-4656
+ while (OwnerScope->getDeclKind() == Decl::LinkageSpec) {
+ OwnerScope = OwnerScope->getParent();
+ }
----------------
vsapsai wrote:
> Looks like `DeclContext::isTransparentContext()` might be relevant here. At least I was able to get the assertion failure
>
> > Assertion failed: (II && "Attempt to mangle unnamed decl."), function getMangledNameImpl, file llvm-project/clang/lib/CodeGen/CodeGenModule.cpp, line 913.
>
> for
> ```lang=c++
> // clang -std=c++17 -fmodules-ts test-modules.cpp
>
> export module M;
> export {
> union {
> int int_val;
> float float_val;
> };
> }
> ```
>
> Also based on `isTransparentContext()` usage, inline namespaces can cause problems. Currently, there are no problems, we have the error
>
> > error: anonymous unions at namespace or global scope must be declared 'static'
>
> and there are no negative consequences (as far as I can tell). According to my bad standard knowledge that should be OK (haven't found non-static anonymous unions to be allowed in this case).
Right, the correct way to spell this is `OwnerScope = Owner->getRedeclContext();`.
================
Comment at: Sema/SemaDecl.cpp:4659-4661
+ (isa<TranslationUnitDecl>(OwnerScope) ||
+ (isa<NamespaceDecl>(OwnerScope) &&
+ cast<NamespaceDecl>(OwnerScope)->getDeclName()))) {
----------------
vsapsai wrote:
> Checked if we need to do the same change s/Owner/OwnerScope/ elsewhere in this method and looks like it is not required. We care if the owner is a Record and we don't allow linkage specification in classes, so skipping linkage scopes doesn't give us anything.
While you're here, `OwnerScope->isFileContext() && !OwnerScope->isInlineNamespace()` might be clearer. Or at least replace the `getDeclName()` with `!isInlineNamespace()`.
Repository:
rC Clang
https://reviews.llvm.org/D45884
More information about the cfe-commits
mailing list