iains marked 2 inline comments as done.
iains added subscribers: vsapsai, dblaikie.
iains added a comment.
So the adjustment to the error message is something I am 50/50 about (IMO it
makes some messages more useful, but maybe not needed in others).
Without the change we get
"cannot export redeclaration 'xxx' here since the previous declaration is not
exported"
So, e.g in C++20 10.2 example 6. every case has the same error message (which
was what prompted me to make the change).
With the change here we now get:
cannot export redeclaration 'f' here since the previous declaration has
internal linkage
cannot export redeclaration 'S' here since the previous declaration has module
linkage
which seems maybe to be more helpful to the user in telling them why.
I hope others can weigh in with an opinion here .. @dblaikie @vsapsai ?
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7832-7833
def err_redeclaration_non_exported : Error <
- "cannot export redeclaration %0 here since the previous declaration is not "
- "exported">;
+ "cannot export redeclaration %0 here since the previous declaration "
+ "%select{is not exported|has internal linkage|has module linkage}1">;
def err_invalid_declarator_global_scope : Error<
----------------
ChuanqiXu wrote:
> I feel this change is not intended?
the change is intentional, but we should maybe decide if it is more or less
helpful to the user.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:1671-1677
+ auto Lk = Old->getFormalLinkage();
+ int S = 0;
+ if (Lk == Linkage::InternalLinkage)
+ S = 1;
+ else if (Lk == Linkage::ModuleLinkage)
+ S = 2;
+ Diag(New->getLocation(), diag::err_redeclaration_non_exported) << New << S;
----------------
ChuanqiXu wrote:
> It looks like that this change isn't reflected in the summary. Although this
> is not bad, I feel it is not good. From the perspective of the user, I feel
> the new message here is a little bit more confusing.
it is reflected in the summary:
"
This adjusts error messages for exports involving redeclarations in modules to
be more specific about the reason that the decl has been rejected.
"
Iet us discuss whether this is more/less useful outside of the code.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122119/new/
https://reviews.llvm.org/D122119
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
