[PATCH] D85390: [Clang] Add note for bad conversion error when expression is of forward-declared type

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Thanks! This looks promising.

In addition to updating existing tests, it would be good to add a test aimed explicitly at this. Maybe you can reduce something from the example in the bug report. Maybe looking at git blame for emitBadConversionNotes() will provide hints for where existing tests for such notes were added.



================
Comment at: clang/lib/Sema/SemaInit.cpp:8710
   }
+  QualType fromType = op->getType();
+  auto *fromDecl = fromType.getTypePtr()->getPointeeCXXRecordDecl();
----------------
Can the reverse situation happen, where it's destType that's forward declared?


================
Comment at: clang/lib/Sema/SemaInit.cpp:8713
+  if (fromDecl && !fromDecl->hasDefinition() && !fromDecl->isInvalidDecl() &&
+      fromDecl->getDeclKind() == Decl::CXXRecord)
+    S.Diag(fromDecl->getLocation(), diag::note_forward_declaration)
----------------
Is the getDeclKind() still necessary even though you're doing getPointeeCXXRecordDecl() above and fromDecl is os type CXXRecordDecl*?


================
Comment at: clang/lib/Sema/SemaInit.cpp:8714
+      fromDecl->getDeclKind() == Decl::CXXRecord)
+    S.Diag(fromDecl->getLocation(), diag::note_forward_declaration)
+        << S.getASTContext().getTagDeclType(fromDecl);
----------------
I think we could add a new note instead of just note_forward_declaration which is more helpful, explaining that maybe the conversion would be valid if the type was defined and not just forward declared.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85390