This is an archive of the discontinued LLVM Phabricator instance.

[Modules TS] Fix namespace visibility
Needs ReviewPublic

Authored by hamzasood on Dec 29 2017, 4:06 AM.

Details

Reviewers
rsmith
boris
bruno
Summary

Namespaces without an explicit export were being mistakenly marked as module private.
This patch fixes the visibility as per [basic.namespace]p1 and adds a test case with previously rejected (yet valid) code.

Diff Detail

Event Timeline

hamzasood created this revision.Dec 29 2017, 4:06 AM
rsmith added inline comments.Apr 27 2018, 4:56 PM
lib/AST/DeclBase.cpp
285

"because they're" -> "because they might be" -- this only applies to Modules TS modules, not to header modules.

lib/Sema/SemaDeclCXX.cpp
8629

This should depend on the kind of module we're in, not whether -fmodules-ts is enabled.

hamzasood updated this revision to Diff 156689.Jul 21 2018, 1:52 PM
  • Don't assume that -fmodules-ts implies that we're in a TS module.
  • Don't ignore private namespaces when calculating the ownership kind.
bruno resigned from this revision.Nov 9 2020, 12:33 PM