On 22/04/2014 00:26, Diego Novillo wrote:
On Mon, Apr 21, 2014 at 7:20 PM, Alp Toker <[email protected]
<mailto:[email protected]>> wrote:
@@ -1084,15 +1100,22 @@
return DB;
}
-inline DiagnosticBuilder
DiagnosticsEngine::Report(SourceLocation Loc,
- unsigned DiagID) {
+inline DiagnosticBuilder
+DiagnosticsEngine::Report(SourceLocation Loc, unsigned
DiagID, StringRef Val) {
assert(CurDiagID == ~0U && "Multiple diagnostics in flight
at once!");
CurDiagLoc = Loc;
CurDiagID = DiagID;
+ FlagNameValue = Val.str();
return DiagnosticBuilder(this);
}
Did you consider adding a DiagnosticBuilder function to append a
FlagNameValue instead of all this?
That would reduce the impact of your patch and improve readability
in the callers. Something like:
Diag(Loc, ID).extendFlag("=mypass") << D;
Yeah, briefly. But it seemed to involve more typing, so I had a slight
preference over the shorter version. It would also mean resetting the
flag name after each call.
I'm not really opposed to the idea, though. I just found the current
version easier to use.
I recommend not modifying the Diag() signature because the feature will
be unavailable to other modules until they each update their own Diag()
signatures introducing latent churn. To get an idea of this:
include/clang/AST/CommentLexer.h: DiagnosticBuilder Diag(SourceLocation
Loc, unsigned DiagID) {
include/clang/AST/CommentParser.h: DiagnosticBuilder
Diag(SourceLocation Loc, unsigned DiagID) {
include/clang/AST/CommentSema.h: DiagnosticBuilder Diag(SourceLocation
Loc, unsigned DiagID) {
include/clang/Driver/Driver.h: DiagnosticBuilder Diag(unsigned DiagID)
const {
include/clang/Lex/Lexer.h: DiagnosticBuilder Diag(const char *Loc,
unsigned DiagID) const;
include/clang/Lex/Preprocessor.h: DiagnosticBuilder Diag(SourceLocation
Loc, unsigned DiagID) const {
include/clang/Lex/Preprocessor.h: DiagnosticBuilder Diag(const Token
&Tok, unsigned DiagID) const {
include/clang/Parse/Parser.h: DiagnosticBuilder Diag(SourceLocation
Loc, unsigned DiagID);
include/clang/Parse/Parser.h: DiagnosticBuilder Diag(const Token &Tok,
unsigned DiagID);
include/clang/Parse/Parser.h: DiagnosticBuilder Diag(unsigned DiagID) {
include/clang/Sema/Sema.h: SemaDiagnosticBuilder Diag(SourceLocation
Loc, unsigned DiagID) {
include/clang/Sema/Sema.h: SemaDiagnosticBuilder Diag(SourceLocation
Loc, const PartialDiagnostic& PD);
include/clang/Serialization/ASTReader.h: DiagnosticBuilder
Diag(unsigned DiagID);
include/clang/Serialization/ASTReader.h: DiagnosticBuilder
Diag(SourceLocation Loc, unsigned DiagID);
And there are probably more cases in other LLVM modules where
DiagnosticBuilders are passed around without access to the original Diag
function that may wish to use this.
Adding a fluent function to DiagnosticBuilder instead of modifying
Diag() signatures is the preferable approach.
Alp.
--
http://www.nuanti.com
the browser experts
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
