owenca wrote:

> > IMO, the best place to handle this is in FormatTokenLexer::getNextToken().
> 
> I see you have much more experience in this part of the codebase, but I have 
> some hangups because I don't understand the implications of doing this move 
> myself. Here's what I'm thinking; are these valid concerns?
> 
> The `FormatTokenLexer` class seems like it does the work of parsing a file 
> into tokens for use downstream by all of the other formatters. What are the 
> implications of running `NumericLiteralCaseFixer` or any other reformatting 
> at this stage for all consumers? Do consumers of lexed `FormatTokens` assume 
> they are receiving a faithful representation of the underlying file? Are 
> there architecture issues regarding separation of concerns that come up if we 
> do formatting directly in the lexing stage?
> 
> Separately, I think that all classes that subclass `TokenAnalyzer` and use 
> `TokenAnalyzer::process()` would wind up calling `FormatTokenLexer::lex()` -- 
> which would end up running `NumericLiteralCaseFixer` reformatting redundantly 
> for each other pass. Please correct me if I'm reading this wrong. I could see 
> how that is still lower overhead than adding a totally separate pass, but it 
> still seems odd to run the same reformatting function many separate times in 
> unrelated passes. Maybe it would be better to add `NumericLiteralCaseFixer` 
> into some other existing pass instead?

You are absolutely right! I was wrong about handling `NumericLiteralCase` in 
`FormatTokenLexer` and totally agree with you that it'd be better to do that in 
an existing pass.

https://github.com/llvm/llvm-project/pull/151590
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to