Manna added inline comments.
================
Comment at: clang/include/clang/Parse/Parser.h:1190
class ParseScopeFlags {
Scope *CurScope;
+ unsigned OldFlags = 0;
----------------
tahonermann wrote:
> shafik wrote:
> > @tahonermann I feel like we should have a default member initializer for
> > any member that by default is not initialized. I realize in this case there
> > currently should be no way for this to be bot initialized but that may not
> > stay true and I don't believe there will be a penalty for doing this since
> > it is initialized in the `mem-initializer-list` and therefore the default
> > init should be omitted when calling the constructor.
> I think that is very reasonable and I'm not opposed to such a policy. Per my
> other comment, the trade off to always initializing is that a default
> initializer can prevent use of tools like ubsan to discover when an
> appropriate (presumably non-default) value has not been assigned. I suspect
> (but have no data to draw on) that adding a default member initializer
> prevents more bugs than would be found by tools like ubsan detecting use of
> an uninitialized data member that should have a (non-default) assigned value.
>
> I'm definitely in favor of using default member initializers over
> `mem-initializer-list`!
>>since it is initialized in the mem-initializer-list and therefore the default
>>init should be omitted when calling the constructor.
Agreed. That is motivated me to add default initialization.
>>he trade off to always initializing is that a default initializer can prevent
>>use of tools like ubsan to discover when an appropriate (presumably
>>non-default) value has not been assigned. I suspect (but have no data to draw
>>on) that adding a default member initializer prevents more bugs than would be
>>found by tools like ubsan detecting use of an uninitialized data member that
>>should have a (non-default) assigned value.
Thanks @tahonermann for the information! i was not aware of this ubsan tool.
@shafik and @tahonermann, thank you for your comments. Do you think we should
hold off these changes for merging?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150744/new/
https://reviews.llvm.org/D150744
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
