iains marked 2 inline comments as done. iains added a comment. In D122119#3398949 <https://reviews.llvm.org/D122119#3398949>, @ChuanqiXu wrote:
> In D122119#3398823 <https://reviews.llvm.org/D122119#3398823>, @iains wrote: > >> 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 ? > > I am OK to wait for opinions from others. Let me talk it a little bit more. > > The first feeling I saw the change is that not every C++ programmer knows > about linkage. OK, it depends on the environment really and every one might > has their own opinion. > > Another thought is that 10.2.6 (http://eel.is/c++draft/module.interface#6) > doesn't talk anything about linkage: > >> A redeclaration of an entity X is implicitly exported if X was introduced by >> an exported declaration; otherwise it shall not be exported. > > So it looks like confusing to talk about linkage this time. In my > imagination, there might be a such situation: > > A programmer met the error when he tries to export a redeclaration which is > internal linkage (maybe a simple const variable). Then the message told him > the internal linkage is not allowed to re-export. Then he removes the const > specifier. Now he meets the error again. It tells that we couldn't export > redeclaration which is module linkage. I guess he would feel bad. Then he > might try to export the first declaration to get passed. However, the `const` > specifier is lost in the case. And in the current message, I guess he would > add export to the first declaration directly after he reads the message. yes, that seems like a reasonable counter argument, and perhaps talking about linkage in a user message is a bit "implementor speak".. ... as I said, I was kind of 50/50 about this - I'll wait 48h for any other comments and then remove the change if there are no votes in favour. 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
