balazske added inline comments.
================
Comment at: clang/include/clang/AST/ASTImportError.h:1
+//===- ASTImporter.h - Import Error while Importing AST -------*- C++ -*-===//
+//
----------------
balazske wrote:
>
================
Comment at: clang/include/clang/AST/ASTImportError.h:22
+
+class ImportError : public llvm::ErrorInfo<ImportError> {
+public:
----------------
phyBrackets wrote:
> balazske wrote:
> > balazske wrote:
> > > Rename to `ASTImportError`.
> > The rename to `ASTImportError` is better in a separate patch. The rename
> > would touch code in much more places and there is a rule to make
> > independent changes in separate patches. Still I think this class should
> > not be called just `ImportError` if it has an own header (it is not an
> > internal-like class of `ASTImporter` as is used to be).
> So, what do you suggest , for this patch should I go with name
> //ImportError// only ? And rename it as //ASTImportError// in a separate
> patch ?
Yes: Do not change the class name here (but header should be called
ASTImportError.h). In a next patch make only a rename.
================
Comment at: clang/include/clang/AST/ASTImportError.h:52
+#endif // LLVM_CLANG_AST_ASTIMPORTERROR_H
\ No newline at end of file
----------------
Please add newline. I do not know if this can cause buildbot failures.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124774/new/
https://reviews.llvm.org/D124774
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
