urnathan added inline comments.
================
Comment at: clang/lib/Sema/SemaTemplate.cpp:2681-2697
+ // Variable used to not diagnose redundant default arguments
+ // from modules.
+ //
+ // [basic.def.odr]/13:
+ // There can be more than one definition of a
+ // ...
+ // default template argument
----------------
while I do approve of good commenting, this is a little wordy and I think
misleading. It is only from global module fragments that you can get duplicate
declarations and those must match default parameters (I think). so perhaps
'IsGlobalModule'? or somehting like that.
Perhaps all of this comment shoould be at the point of checking? After all the
preceding comment mentions 'variables used to ...'?
================
Comment at: clang/lib/Sema/SemaTemplate.cpp:2852
- if (RedundantDefaultArg) {
+ if (RedundantDefaultArg && !IsOldParamFromModule) {
// C++ [temp.param]p12:
----------------
Yes, I think this is a better place for the comment. If there are duplicates
we need to check they are the same -- pedantically the same tokens, but at
least AST equivalence?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118034/new/
https://reviews.llvm.org/D118034
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
