[PATCH] D53847: [C++2a] P0634r3: Down with typename!

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

Maybe I missed it but I don't see a test that covers temp.res p5 <https://eel.is/c++draft/temp.res#general-5> e.g.:

  template <class T> void f(int i) {
    T::x * i;  // This will be assumed to be the expression
               //   T::x multiplied by i
               // Not a declaration of variable i of 
               //   type pointer to T::x?
  }
  
  struct Foo {
   typedef int x;
  };
  
  struct Bar {
    static int const x = 5;
  };
  
  int main() {
    f<Bar>(1);          // OK
    f<Foo>(1);          // error: Foo::x is a type
  }



================
Comment at: clang/include/clang/Sema/DeclSpec.h:1918
+  /// Lookup result of declarator, if any.
+  std::unique_ptr<LookupResult> PrevLookupResult;
+
----------------
Why do we need this one shot previous lookup result? I am struggling to see the connection between `isDeclaratorFunctionLike(...)` and where we check `hasPrevLookupResult(...)` and why it matter.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:6479
+        else if (D.getContext() == DeclaratorContext::Member) {
+          llvm::dbgs() << __FUNCTION__ << ": " << __LINE__ << "\n";
+          AllowImplicitTypename = ImplicitTypenameContext::Yes;
----------------
Looks like there are a few of these left over as debugging lines, the should removed.


================
Comment at: clang/lib/Sema/DeclSpec.cpp:1502-1504
+  LookupResult LR = std::move(*PrevLookupResult);
+  PrevLookupResult.reset();
+  return LR;
----------------



================
Comment at: clang/test/CXX/temp/temp.res/p5.cpp:1
+// RUN: %clang_cc1 -std=c++20 -pedantic -verify %s
+
----------------
It looks like this is [temp.res p4 now](https://eel.is/c++draft/temp.res#general-4)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D53847