rsmith added inline comments.
================
Comment at: clang/lib/Sema/SemaTemplate.cpp:2651
+ Expr *DefaultArgumentY = NTTPY->getDefaultArgument()->IgnoreImpCasts();
+ ODRHash X, Y;
+ X.AddStmt(DefaultArgumentX);
----------------
ODR hashing is intended to produce a hash that's stable across compilations (so
it can't use pointer comparisons to check types are identical, for example).
For use cases that don't need a hash code that's stable across processes, using
`Stmt::Profile` should be preferred. Can you use that here?
================
Comment at: clang/lib/Sema/SemaTemplate.cpp:2720-2728
+ // Variables used to diagnose redundant default arguments in the same
+ // translation unit.
bool RedundantDefaultArg = false;
+ // Variables used to diagnose inconsitent default arguments in different
+ // translation unit.
+ bool InconsistentDefaultArg = false;
+ // Variables used to diagnose the module name of the old param in case the
----------------
Please rephrase these comments to remove the "Variables used to" part and to
explain instead what value is stored in the variable. (Eg: "Whether we've seen
a duplicate default argument in the same translation unit.")
================
Comment at: clang/lib/Sema/SemaTemplate.cpp:2762
SawDefaultArgument = true;
- RedundantDefaultArg = true;
+ if (!OldTypeParm->getImportedOwningModule())
+ RedundantDefaultArg = true;
----------------
This is the wrong thing to check -- whether the owning module is imported (from
an AST file) is irrelevant. We might parse multiple translation units in the
same compilation, and we might split a single translation unit into multiple
compilations (eg with a precompiled preamble) resulting in some of the same
translation unit being in an AST file. What matters is whether the declaration
is in the current translation unit or a different translation unit.
I'm not sure what the best way to detect that is currently. You might be able
to just compare `getOwningModule()` of the two declarations, but I don't think
that will properly handle cases in the global module fragment, or cases where
one template is in the primary module interface unit and another is in a module
implementation unit of the same module.
Ideally I would like for us to create one `TranslationUnitDecl` per translation
unit, rather than shoving all imported code into the translation unit of the
importer, but that's a moderately large change, which I'd like to not block
this change on if we can avoid it. But I think that's the right longer-term
approach. Then here we would just need to find the lexically-enclosing
`TranslationUnitDecl` for each template parameter and see if they're the same.
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
