[PATCH] D82738: [clang] Fix a null-NSS-access crash in DependentNameType.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:4869
               LookupResult::NotFoundInCurrentInstantiation) {
+        if (SS.isEmpty())
+          // bail out if we don't have a NNS, this could be happened during
----------------
two concerns here:
 - it's not clear to me whether it's best to recover from this failed invariant or make a change elsewhere to ensure it holds
 - IIUC we should only return true if an error has been emitted. We've got some suggestion that it already has, but no smoking gun - maybe there are cases we've missed. (The failure mode here is clang exits with an error status but prints no errors)


================
Comment at: clang/test/Parser/cxx-template-decl.cpp:296
+class UsingImpl {};
+class AddObservation {
+  using Using =
----------------
If I understand right, AddObservationFn gets typo-corrected to AddObservation (the function). When we go to check whether it's valid, we look up the name and get the type instead (injected-class-name). so we figure "missing typename" but actually everything's just totally confused.

The "best" solution I guess would be not allowing typo-correction to the shadowed name.
But I think the next best thing is just not to allow "insert typename and recover" if the expression we're inserting around already has errors - this seems like we have low confidence at this point.

So does requiring   !Arg.getAsExpr().containsErrors()  in the check for inserting typename work?
My hope is this would result in "undeclared identifier AddObservationFn, did you mean AddObservation" ... "template argument must be a type".
(Which is "wrong" in the sense that AddObservation is a type, but that bug is in the typo-correction logic, not here)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82738