<div dir="ltr"><div dir="ltr">With this patch some ranges just come back empty. I rolled the change back in r351209, which also includes a test case that shows the empty ranges with this patch.</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Jan 10, 2019 at 10:26 PM Aaron Ballman via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: aaronballman<br>
Date: Thu Jan 10 13:22:13 2019<br>
New Revision: 350891<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=350891&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=350891&view=rev</a><br>
Log:<br>
Correct the source range returned from preprocessor callbacks.<br>
<br>
This adjusts the source range passed in to the preprocessor callbacks to only include the condition range itself, rather than all of the conditionally skipped tokens.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Lex/Preprocessor.h<br>
cfe/trunk/lib/Lex/PPDirectives.cpp<br>
cfe/trunk/lib/Lex/PPExpressions.cpp<br>
cfe/trunk/unittests/Lex/PPCallbacksTest.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Lex/Preprocessor.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=350891&r1=350890&r2=350891&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=350891&r1=350890&r2=350891&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Lex/Preprocessor.h (original)<br>
+++ cfe/trunk/include/clang/Lex/Preprocessor.h Thu Jan 10 13:22:13 2019<br>
@@ -1816,8 +1816,8 @@ public:<br>
void CheckEndOfDirective(const char *DirType, bool EnableMacros = false);<br>
<br>
/// Read and discard all tokens remaining on the current line until<br>
- /// the tok::eod token is found.<br>
- void DiscardUntilEndOfDirective();<br>
+ /// the tok::eod token is found. Returns the range of the skipped tokens.<br>
+ SourceRange DiscardUntilEndOfDirective();<br>
<br>
/// Returns true if the preprocessor has seen a use of<br>
/// __DATE__ or __TIME__ in the file so far.<br>
@@ -1982,6 +1982,9 @@ private:<br>
<br>
/// True if the expression contained identifiers that were undefined.<br>
bool IncludedUndefinedIds;<br>
+<br>
+ /// The source range for the expression.<br>
+ SourceRange ExprRange;<br>
};<br>
<br>
/// Evaluate an integer constant expression that may occur after a<br>
<br>
Modified: cfe/trunk/lib/Lex/PPDirectives.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=350891&r1=350890&r2=350891&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=350891&r1=350890&r2=350891&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)<br>
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Thu Jan 10 13:22:13 2019<br>
@@ -79,12 +79,18 @@ Preprocessor::AllocateVisibilityMacroDir<br>
<br>
/// Read and discard all tokens remaining on the current line until<br>
/// the tok::eod token is found.<br>
-void Preprocessor::DiscardUntilEndOfDirective() {<br>
+SourceRange Preprocessor::DiscardUntilEndOfDirective() {<br>
Token Tmp;<br>
- do {<br>
- LexUnexpandedToken(Tmp);<br>
+ SourceRange Res;<br>
+<br>
+ LexUnexpandedToken(Tmp);<br>
+ Res.setBegin(Tmp.getLocation());<br>
+ while (Tmp.isNot(tok::eod)) {<br>
assert(Tmp.isNot(tok::eof) && "EOF seen while discarding directive tokens");<br>
- } while (Tmp.isNot(tok::eod));<br>
+ LexUnexpandedToken(Tmp);<br>
+ }<br>
+ Res.setEnd(Tmp.getLocation());<br>
+ return Res;<br>
}<br>
<br>
/// Enumerates possible cases of #define/#undef a reserved identifier.<br>
@@ -538,19 +544,19 @@ void Preprocessor::SkipExcludedCondition<br>
if (CondInfo.WasSkipping || CondInfo.FoundNonSkip) {<br>
DiscardUntilEndOfDirective();<br>
} else {<br>
- const SourceLocation CondBegin = CurPPLexer->getSourceLocation();<br>
// Restore the value of LexingRawMode so that identifiers are<br>
// looked up, etc, inside the #elif expression.<br>
assert(CurPPLexer->LexingRawMode && "We have to be skipping here!");<br>
CurPPLexer->LexingRawMode = false;<br>
IdentifierInfo *IfNDefMacro = nullptr;<br>
- const bool CondValue = EvaluateDirectiveExpression(IfNDefMacro).Conditional;<br>
+ DirectiveEvalResult DER = EvaluateDirectiveExpression(IfNDefMacro);<br>
+ const bool CondValue = DER.Conditional;<br>
CurPPLexer->LexingRawMode = true;<br>
if (Callbacks) {<br>
- const SourceLocation CondEnd = CurPPLexer->getSourceLocation();<br>
- Callbacks->Elif(Tok.getLocation(),<br>
- SourceRange(CondBegin, CondEnd),<br>
- (CondValue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False), CondInfo.IfLoc);<br>
+ Callbacks->Elif(<br>
+ Tok.getLocation(), DER.ExprRange,<br>
+ (CondValue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False),<br>
+ CondInfo.IfLoc);<br>
}<br>
// If this condition is true, enter it!<br>
if (CondValue) {<br>
@@ -1116,19 +1122,24 @@ void Preprocessor::HandleLineDirective()<br>
; // ok<br>
else if (StrTok.isNot(tok::string_literal)) {<br>
Diag(StrTok, diag::err_pp_line_invalid_filename);<br>
- return DiscardUntilEndOfDirective();<br>
+ DiscardUntilEndOfDirective();<br>
+ return;<br>
} else if (StrTok.hasUDSuffix()) {<br>
Diag(StrTok, diag::err_invalid_string_udl);<br>
- return DiscardUntilEndOfDirective();<br>
+ DiscardUntilEndOfDirective();<br>
+ return;<br>
} else {<br>
// Parse and validate the string, converting it into a unique ID.<br>
StringLiteralParser Literal(StrTok, *this);<br>
assert(Literal.isAscii() && "Didn't allow wide strings in");<br>
- if (Literal.hadError)<br>
- return DiscardUntilEndOfDirective();<br>
+ if (Literal.hadError) {<br>
+ DiscardUntilEndOfDirective();<br>
+ return;<br>
+ }<br>
if (Literal.Pascal) {<br>
Diag(StrTok, diag::err_pp_linemarker_invalid_filename);<br>
- return DiscardUntilEndOfDirective();<br>
+ DiscardUntilEndOfDirective();<br>
+ return;<br>
}<br>
FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());<br>
<br>
@@ -1261,19 +1272,24 @@ void Preprocessor::HandleDigitDirective(<br>
FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());<br>
} else if (StrTok.isNot(tok::string_literal)) {<br>
Diag(StrTok, diag::err_pp_linemarker_invalid_filename);<br>
- return DiscardUntilEndOfDirective();<br>
+ DiscardUntilEndOfDirective();<br>
+ return;<br>
} else if (StrTok.hasUDSuffix()) {<br>
Diag(StrTok, diag::err_invalid_string_udl);<br>
- return DiscardUntilEndOfDirective();<br>
+ DiscardUntilEndOfDirective();<br>
+ return;<br>
} else {<br>
// Parse and validate the string, converting it into a unique ID.<br>
StringLiteralParser Literal(StrTok, *this);<br>
assert(Literal.isAscii() && "Didn't allow wide strings in");<br>
- if (Literal.hadError)<br>
- return DiscardUntilEndOfDirective();<br>
+ if (Literal.hadError) {<br>
+ DiscardUntilEndOfDirective();<br>
+ return;<br>
+ }<br>
if (Literal.Pascal) {<br>
Diag(StrTok, diag::err_pp_linemarker_invalid_filename);<br>
- return DiscardUntilEndOfDirective();<br>
+ DiscardUntilEndOfDirective();<br>
+ return;<br>
}<br>
FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());<br>
<br>
@@ -1343,7 +1359,8 @@ void Preprocessor::HandleIdentSCCSDirect<br>
<br>
if (StrTok.hasUDSuffix()) {<br>
Diag(StrTok, diag::err_invalid_string_udl);<br>
- return DiscardUntilEndOfDirective();<br>
+ DiscardUntilEndOfDirective();<br>
+ return;<br>
}<br>
<br>
// Verify that there is nothing after the string, other than EOD.<br>
@@ -2783,10 +2800,8 @@ void Preprocessor::HandleIfDirective(Tok<br>
<br>
// Parse and evaluate the conditional expression.<br>
IdentifierInfo *IfNDefMacro = nullptr;<br>
- const SourceLocation ConditionalBegin = CurPPLexer->getSourceLocation();<br>
const DirectiveEvalResult DER = EvaluateDirectiveExpression(IfNDefMacro);<br>
const bool ConditionalTrue = DER.Conditional;<br>
- const SourceLocation ConditionalEnd = CurPPLexer->getSourceLocation();<br>
<br>
// If this condition is equivalent to #ifndef X, and if this is the first<br>
// directive seen, handle it for the multiple-include optimization.<br>
@@ -2799,9 +2814,9 @@ void Preprocessor::HandleIfDirective(Tok<br>
}<br>
<br>
if (Callbacks)<br>
- Callbacks->If(IfToken.getLocation(),<br>
- SourceRange(ConditionalBegin, ConditionalEnd),<br>
- (ConditionalTrue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False));<br>
+ Callbacks->If(<br>
+ IfToken.getLocation(), DER.ExprRange,<br>
+ (ConditionalTrue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False));<br>
<br>
// Should we include the stuff contained by this directive?<br>
if (PPOpts->SingleFileParseMode && DER.IncludedUndefinedIds) {<br>
@@ -2894,9 +2909,7 @@ void Preprocessor::HandleElifDirective(T<br>
// #elif directive in a non-skipping conditional... start skipping.<br>
// We don't care what the condition is, because we will always skip it (since<br>
// the block immediately before it was included).<br>
- const SourceLocation ConditionalBegin = CurPPLexer->getSourceLocation();<br>
- DiscardUntilEndOfDirective();<br>
- const SourceLocation ConditionalEnd = CurPPLexer->getSourceLocation();<br>
+ SourceRange ConditionRange = DiscardUntilEndOfDirective();<br>
<br>
PPConditionalInfo CI;<br>
if (CurPPLexer->popConditionalLevel(CI)) {<br>
@@ -2912,8 +2925,7 @@ void Preprocessor::HandleElifDirective(T<br>
if (CI.FoundElse) Diag(ElifToken, diag::pp_err_elif_after_else);<br>
<br>
if (Callbacks)<br>
- Callbacks->Elif(ElifToken.getLocation(),<br>
- SourceRange(ConditionalBegin, ConditionalEnd),<br>
+ Callbacks->Elif(ElifToken.getLocation(), ConditionRange,<br>
PPCallbacks::CVK_NotEvaluated, CI.IfLoc);<br>
<br>
if (PPOpts->SingleFileParseMode && !CI.FoundNonSkip) {<br>
<br>
Modified: cfe/trunk/lib/Lex/PPExpressions.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPExpressions.cpp?rev=350891&r1=350890&r2=350891&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPExpressions.cpp?rev=350891&r1=350890&r2=350891&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Lex/PPExpressions.cpp (original)<br>
+++ cfe/trunk/lib/Lex/PPExpressions.cpp Thu Jan 10 13:22:13 2019<br>
@@ -152,8 +152,8 @@ static bool EvaluateDefined(PPValue &Res<br>
return true;<br>
}<br>
// Consume the ).<br>
- Result.setEnd(PeekTok.getLocation());<br>
PP.LexNonComment(PeekTok);<br>
+ Result.setEnd(PeekTok.getLocation());<br>
} else {<br>
// Consume identifier.<br>
Result.setEnd(PeekTok.getLocation());<br>
@@ -863,7 +863,7 @@ Preprocessor::EvaluateDirectiveExpressio<br>
<br>
// Restore 'DisableMacroExpansion'.<br>
DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;<br>
- return {ResVal.Val != 0, DT.IncludedUndefinedIds};<br>
+ return {ResVal.Val != 0, DT.IncludedUndefinedIds, ResVal.getRange()};<br>
}<br>
<br>
// Otherwise, we must have a binary operator (e.g. "#if 1 < 2"), so parse the<br>
@@ -876,7 +876,7 @@ Preprocessor::EvaluateDirectiveExpressio<br>
<br>
// Restore 'DisableMacroExpansion'.<br>
DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;<br>
- return {false, DT.IncludedUndefinedIds};<br>
+ return {false, DT.IncludedUndefinedIds, ResVal.getRange()};<br>
}<br>
<br>
// If we aren't at the tok::eod token, something bad happened, like an extra<br>
@@ -888,5 +888,5 @@ Preprocessor::EvaluateDirectiveExpressio<br>
<br>
// Restore 'DisableMacroExpansion'.<br>
DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;<br>
- return {ResVal.Val != 0, DT.IncludedUndefinedIds};<br>
+ return {ResVal.Val != 0, DT.IncludedUndefinedIds, ResVal.getRange()};<br>
}<br>
<br>
Modified: cfe/trunk/unittests/Lex/PPCallbacksTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/PPCallbacksTest.cpp?rev=350891&r1=350890&r2=350891&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/PPCallbacksTest.cpp?rev=350891&r1=350890&r2=350891&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Lex/PPCallbacksTest.cpp (original)<br>
+++ cfe/trunk/unittests/Lex/PPCallbacksTest.cpp Thu Jan 10 13:22:13 2019<br>
@@ -65,6 +65,29 @@ public:<br>
SrcMgr::CharacteristicKind FileType;<br>
};<br>
<br>
+class CondDirectiveCallbacks : public PPCallbacks {<br>
+public:<br>
+ struct Result {<br>
+ SourceRange ConditionRange;<br>
+ ConditionValueKind ConditionValue;<br>
+<br>
+ Result(SourceRange R, ConditionValueKind K)<br>
+ : ConditionRange(R), ConditionValue(K) {}<br>
+ };<br>
+<br>
+ std::vector<Result> Results;<br>
+<br>
+ void If(SourceLocation Loc, SourceRange ConditionRange,<br>
+ ConditionValueKind ConditionValue) override {<br>
+ Results.emplace_back(ConditionRange, ConditionValue);<br>
+ }<br>
+<br>
+ void Elif(SourceLocation Loc, SourceRange ConditionRange,<br>
+ ConditionValueKind ConditionValue, SourceLocation IfLoc) override {<br>
+ Results.emplace_back(ConditionRange, ConditionValue);<br>
+ }<br>
+};<br>
+<br>
// Stub to collect data from PragmaOpenCLExtension callbacks.<br>
class PragmaOpenCLExtensionCallbacks : public PPCallbacks {<br>
public:<br>
@@ -137,6 +160,15 @@ protected:<br>
return StringRef(B, E - B);<br>
}<br>
<br>
+ StringRef GetSourceStringToEnd(CharSourceRange Range) {<br>
+ const char *B = SourceMgr.getCharacterData(Range.getBegin());<br>
+ const char *E = SourceMgr.getCharacterData(Range.getEnd());<br>
+<br>
+ return StringRef(<br>
+ B,<br>
+ E - B + Lexer::MeasureTokenLength(Range.getEnd(), SourceMgr, LangOpts));<br>
+ }<br>
+<br>
// Run lexer over SourceText and collect FilenameRange from<br>
// the InclusionDirective callback.<br>
CharSourceRange InclusionDirectiveFilenameRange(const char *SourceText,<br>
@@ -199,6 +231,36 @@ protected:<br>
return Callbacks;<br>
}<br>
<br>
+ std::vector<CondDirectiveCallbacks::Result><br>
+ DirectiveExprRange(StringRef SourceText) {<br>
+ TrivialModuleLoader ModLoader;<br>
+ MemoryBufferCache PCMCache;<br>
+ std::unique_ptr<llvm::MemoryBuffer> Buf =<br>
+ llvm::MemoryBuffer::getMemBuffer(SourceText);<br>
+ SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));<br>
+ HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,<br>
+ Diags, LangOpts, Target.get());<br>
+ Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,<br>
+ SourceMgr, PCMCache, HeaderInfo, ModLoader,<br>
+ /*IILookup =*/nullptr,<br>
+ /*OwnsHeaderSearch =*/false);<br>
+ PP.Initialize(*Target);<br>
+ auto *Callbacks = new CondDirectiveCallbacks;<br>
+ PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(Callbacks));<br>
+<br>
+ // Lex source text.<br>
+ PP.EnterMainSourceFile();<br>
+<br>
+ while (true) {<br>
+ Token Tok;<br>
+ PP.Lex(Tok);<br>
+ if (Tok.is(tok::eof))<br>
+ break;<br>
+ }<br>
+<br>
+ return Callbacks->Results;<br>
+ }<br>
+<br>
PragmaOpenCLExtensionCallbacks::CallbackParameters<br>
PragmaOpenCLExtensionCall(const char *SourceText) {<br>
LangOptions OpenCLLangOpts;<br>
@@ -368,4 +430,59 @@ TEST_F(PPCallbacksTest, OpenCLExtensionP<br>
ASSERT_EQ(ExpectedState, Parameters.State);<br>
}<br>
<br>
-} // anonoymous namespace<br>
+TEST_F(PPCallbacksTest, DirectiveExprRanges) {<br>
+ const auto &Results1 = DirectiveExprRange("#if FLUZZY_FLOOF\n#endif\n");<br>
+ EXPECT_EQ(Results1.size(), 1);<br>
+ EXPECT_EQ(<br>
+ GetSourceStringToEnd(CharSourceRange(Results1[0].ConditionRange, false)),<br>
+ "FLUZZY_FLOOF");<br>
+<br>
+ const auto &Results2 = DirectiveExprRange("#if 1 + 4 < 7\n#endif\n");<br>
+ EXPECT_EQ(Results2.size(), 1);<br>
+ EXPECT_EQ(<br>
+ GetSourceStringToEnd(CharSourceRange(Results2[0].ConditionRange, false)),<br>
+ "1 + 4 < 7");<br>
+<br>
+ const auto &Results3 = DirectiveExprRange("#if 1 + \\\n 2\n#endif\n");<br>
+ EXPECT_EQ(Results3.size(), 1);<br>
+ EXPECT_EQ(<br>
+ GetSourceStringToEnd(CharSourceRange(Results3[0].ConditionRange, false)),<br>
+ "1 + \\\n 2");<br>
+<br>
+ const auto &Results4 = DirectiveExprRange("#if 0\n#elif FLOOFY\n#endif\n");<br>
+ EXPECT_EQ(Results4.size(), 2);<br>
+ EXPECT_EQ(<br>
+ GetSourceStringToEnd(CharSourceRange(Results4[0].ConditionRange, false)),<br>
+ "0");<br>
+ EXPECT_EQ(<br>
+ GetSourceStringToEnd(CharSourceRange(Results4[1].ConditionRange, false)),<br>
+ "FLOOFY");<br>
+<br>
+ const auto &Results5 = DirectiveExprRange("#if 1\n#elif FLOOFY\n#endif\n");<br>
+ EXPECT_EQ(Results5.size(), 2);<br>
+ EXPECT_EQ(<br>
+ GetSourceStringToEnd(CharSourceRange(Results5[0].ConditionRange, false)),<br>
+ "1");<br>
+ EXPECT_EQ(<br>
+ GetSourceStringToEnd(CharSourceRange(Results5[1].ConditionRange, false)),<br>
+ "FLOOFY");<br>
+<br>
+ const auto &Results6 =<br>
+ DirectiveExprRange("#if defined(FLUZZY_FLOOF)\n#endif\n");<br>
+ EXPECT_EQ(Results6.size(), 1);<br>
+ EXPECT_EQ(<br>
+ GetSourceStringToEnd(CharSourceRange(Results6[0].ConditionRange, false)),<br>
+ "defined(FLUZZY_FLOOF)");<br>
+<br>
+ const auto &Results7 =<br>
+ DirectiveExprRange("#if 1\n#elif defined(FLOOFY)\n#endif\n");<br>
+ EXPECT_EQ(Results7.size(), 2);<br>
+ EXPECT_EQ(<br>
+ GetSourceStringToEnd(CharSourceRange(Results7[0].ConditionRange, false)),<br>
+ "1");<br>
+ EXPECT_EQ(<br>
+ GetSourceStringToEnd(CharSourceRange(Results7[1].ConditionRange, false)),<br>
+ "defined(FLOOFY)");<br>
+}<br>
+<br>
+} // namespace<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>