[all-commits] [llvm/llvm-project] 487967: [Modules] Don't replace local declarations with ex...

[Chuanqi Xu via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 487967af82053cd08022635a2ff768385d936c80
      https://github.com/llvm/llvm-project/commit/487967af82053cd08022635a2ff768385d936c80
  Author: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
  Date:   2024-04-28 (Sun, 28 Apr 2024)

  Changed paths:
    M clang/include/clang/AST/DeclContextInternals.h
    A clang/test/Modules/pr88400.cppm

  Log Message:
  -----------
  [Modules] Don't replace local declarations with external declaration with lower visibility

Close https://github.com/llvm/llvm-project/issues/88400

For the reproducer:

```
//--- header.h

namespace N {
    template<typename T>
    concept X = true;

    template<X T>
    class Y {
    public:
        template<X U>
        friend class Y;
    };

    inline Y<int> x;
}

//--- bar.cppm
module;
export module bar;
namespace N {
    // To make sure N::Y won't get elided.
    using N::x;
}

//--- foo.cc
// expected-no-diagnostics
import bar;
void y() {
    N::Y<int> y{};
};
```

it will crash. The root cause is that in
`StoredDeclsList::replaceExternalDecls`, we will replace the
existing declarations with external declarations.

Then for the reproducer, the redecl chain for Y is like:

```
Y (Local) -> Y (Local, friend) -> Y (Imported) -> Y(Imported, friend)
```

Before the lookup, the stored lookup result is `Y(Local)` then we find
`Y(Imported)`. And now we repalce `Y(Local)` with `Y(Imported)`. But
`Y(Imported)` is not visible. So we tried to find if there is any
redeclarations visible but we find `Y(Local, friend)`, then problem
happens.

The solution is try to avoid the replace to happen if the external
declaration has lower visibility then we can always find the local
declarations. This may help the lookup performance slightly.

Also I found the implementation of
`StoredDeclsList::replaceExternalDecls` is not efficiency. It has an
`O(n*m)` complexities. But let's improve that in the future.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications