omtcyfz added inline comments.

================
Comment at: lib/Analysis/CloneDetection.cpp:436
@@ +435,3 @@
+    if (IsInMacro) {
+      Signature.Complexity = 0;
+    }
----------------
NoQ wrote:
> omtcyfz wrote:
> > Do I understand correctly that a code generated by a macro doesn't affect 
> > "complexity" at all then?
> > 
> > ```
> > TEST_F(QueryParserTest, Complete) {
> >   std::vector<llvm::LineEditor::Completion> Comps =
> >       QueryParser::complete("", 0, QS);
> >   ASSERT_EQ(6u, Comps.size());
> >   EXPECT_EQ("help ", Comps[0].TypedText);
> >   EXPECT_EQ("help", Comps[0].DisplayText);
> >   EXPECT_EQ("let ", Comps[1].TypedText);
> >   EXPECT_EQ("let", Comps[1].DisplayText);
> >   EXPECT_EQ("match ", Comps[2].TypedText);
> >   EXPECT_EQ("match", Comps[2].DisplayText);
> >   EXPECT_EQ("set ", Comps[3].TypedText);
> >   EXPECT_EQ("set", Comps[3].DisplayText);
> >   EXPECT_EQ("unlet ", Comps[4].TypedText);
> >   EXPECT_EQ("unlet", Comps[4].DisplayText);
> >   EXPECT_EQ("quit", Comps[5].DisplayText);
> >   EXPECT_EQ("quit ", Comps[5].TypedText);
> > 
> >   Comps = QueryParser::complete("set o", 5, QS);
> >   ASSERT_EQ(1u, Comps.size());
> >   EXPECT_EQ("utput ", Comps[0].TypedText);
> >   EXPECT_EQ("output", Comps[0].DisplayText);
> > 
> >   Comps = QueryParser::complete("match while", 11, QS);
> >   ASSERT_EQ(1u, Comps.size());
> >   EXPECT_EQ("Stmt(", Comps[0].TypedText);
> >   EXPECT_EQ("Matcher<Stmt> whileStmt(Matcher<WhileStmt>...)",
> >             Comps[0].DisplayText);
> > }
> > ```
> > 
> > This is an actual piece of code from 
> > `extra/unittests/clang-query/QueryParserTest.cpp`. Yes, it is a test, but 
> > it still is a nice example of how many macros can be found in code 
> > (especially if we are talking about pure C or some weird C++).
> > 
> > Thus, I think it is reasonable to treat macro invocation as a 
> > `1`-"complexity" node.
> This "0" is not for the macro itself, but for the statements into which it 
> expands. Macro itself is not a statement. If we put "1" here, it would 
> produce a lot more complexity than you want.
> 
> That said, it's a good idea to treat every macro as a "complexity-1" 
> statement, just need to figure out how to implement that correctly :)
> 
> Perhaps scan the source range of the sequence for how many different macro 
> expansions are included, and add that number to complexity(?)
> This "0" is not for the macro itself, but for the statements into which it 
> expands. Macro itself is not a statement. If we put "1" here, it would 
> produce a lot more complexity than you want.

Sure, I understand that, this is why I didn't suggest putting `1` there.

> Perhaps scan the source range of the sequence for how many different macro 
> expansions are included, and add that number to complexity(?)

Yes, this is exactly the solution that would work. Since macros aren't in the 
AST we'd need to get through SourceRange anyway.


https://reviews.llvm.org/D23316



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to