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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to