[PATCH] D154503: [Sema] Fix handling of functions that hide classes

[John Brawn via Phabricator via cfe-commits]

john.brawn added inline comments.


================
Comment at: clang/lib/Sema/SemaLookup.cpp:507
 
+  // C++ [basic.scope.hiding]p2:
+  //   A class name or enumeration name can be hidden by the name of
----------------
shafik wrote:
> This section does not exist anymore, it was replaced in [p1787](https://wg21.link/p1787) which resolved a very large number of DRs and reflector discussions. I have not fully digested the paper myself but this change should reflect the new wording as it exists in the current draft. 
It looks like https://eel.is/c++draft/basic.lookup#general-4 is the same thing but worded differently. That draft hasn't gone into a published standard though, and could change before it gets published, and the same section is referenced elsewhere in this file (and there are probably other references in this file to parts of the standard that will get changed in the next version), so I think it would make more sense to change all such comments at once when that change has gone into a published version of the standard.


================
Comment at: clang/test/SemaCXX/using-hiding.cpp:20
+
+// Using declaration causes A::X to be hidden, so X is not ambiguous.
+namespace Test2 {
----------------
shafik wrote:
> I think [namespace.udecl p10](https://eel.is/c++draft/namespace.udecl#10) disagrees, specifically:
> 
> ```
>  using A::g;                           // error: conflicts with B​::​g
> ```
> 
> but I may be misreading CC @Endill who has been looking at p1787r6 in details where a lot of this wording changed.
I think that would mean that the using-declaration is ill-formed (i.e. we would give an error before we even arrived at looking up the name X). I'll try to adjust these tests so that they won't fail due to this when clang implements this behaviour.


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

https://reviews.llvm.org/D154503