aprantl added inline comments.
================
Comment at: clang/test/CodeGenObjCXX/encode.mm:93-94
// FIXME: This difference is due to D76801. It was probably an unintentional
change. Maybe we want to undo it?
- // CHECKCXX98: @_ZN11rdar93574002ggE ={{.*}} constant [49 x i8]
c"{vector<float, rdar9357400::fixed<4, -1> >=[4f]}\00"
- // CHECKCXX20: @_ZN11rdar93574002ggE ={{.*}} constant [48 x i8]
c"{vector<float, rdar9357400::fixed<4, -1>>=[4f]}\00"
+ // CHECKCXX98: @_ZN11rdar93574002ggE ={{.*}} constant [45 x i8]
c"{vector<float, rdar9357400::fixed<4> >=[4f]}\00"
+ // CHECKCXX20: @_ZN11rdar93574002ggE ={{.*}} constant [44 x i8]
c"{vector<float, rdar9357400::fixed<4>>=[4f]}\00"
extern const char gg[] = @encode(vector4f);
----------------
Michael137 wrote:
> Michael137 wrote:
> > dblaikie wrote:
> > > @aprantl any idea if this is good/OK? (I guess it probably is - but maybe
> > > these strings were never meant to ignore/suppress default arguments of
> > > any kind? or maybe this is an ABI sort of thing where it suppressing some
> > > but not others is now unchangeable?)
> > Good point. There was a thread on the cfe mailing list a while ago about
> > the last time this broke:
> > https://lists.llvm.org/pipermail/cfe-dev/2020-November/067194.html
> >
> > This was @rsmith's stance:
> > ```
> > I think some of the other recent TypePrinter changes might also risk
> > changing the @encode output. Generally it seems unwise for @encode to be
> > using the type pretty-printer if it wants to be ABI-stable; I don't think
> > it's reasonable to expect any guarantees as to the stability of
> > pretty-printed type names. I think USR generation suffers from similar
> > problems; it too uses the type pretty-printer to generate
> > supposedly-ABI-stable keys in at least some cases.
> > ```
> see https://reviews.llvm.org/D90622
To me it really looks like the intention of the feature is to not substitute
default parameters. But if we stop doing this now it will likely result in a
surprising code size increase, that may not be considered worth it compared to
the risk of breaking ABI by changing a default template parameter.
As far as this patch is concerned, it's neutral to this decision (which may not
have been a conscious one).
It's certainly not good that every type printer change is an ABI break.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139986/new/
https://reviews.llvm.org/D139986
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
